From 3d36e15f674d8e3f94e30bc1a5dce2b3ae5122e5 Mon Sep 17 00:00:00 2001 From: Eyejoker Date: Wed, 3 Jun 2026 01:14:39 +0800 Subject: [PATCH] Fix recurring Codex unavailable in paired rooms (#212) * fix: avoid codex leases for claude readonly sessions * fix: restore codex lease quality budget * fix: preserve owner finalization on codex failure * fix: wake owner after arbiter codex failure * chore: format readonly codex lease fix --- src/agent-runner-environment.test.ts | 39 +++- src/agent-runner-environment.ts | 59 +++--- src/agent-runner-process.ts | 68 ++++--- src/agent-runner.test.ts | 135 +++++++------- src/message-agent-executor-paired.ts | 1 + src/message-agent-executor-rules.test.ts | 10 +- src/message-agent-executor-rules.ts | 8 +- src/message-runtime-follow-up.ts | 32 ++-- src/owner-final-codex-unavailable.test.ts | 179 +++++++++++++++++++ src/paired-arbiter-codex-owner-wake.test.ts | 81 +++++++++ src/paired-execution-context-arbiter.ts | 16 +- src/paired-execution-context-owner.ts | 66 ++++++- src/paired-execution-context-routing.test.ts | 25 ++- 13 files changed, 559 insertions(+), 160 deletions(-) create mode 100644 src/owner-final-codex-unavailable.test.ts create mode 100644 src/paired-arbiter-codex-owner-wake.test.ts diff --git a/src/agent-runner-environment.test.ts b/src/agent-runner-environment.test.ts index ae54aa1..81d1f02 100644 --- a/src/agent-runner-environment.test.ts +++ b/src/agent-runner-environment.test.ts @@ -15,7 +15,13 @@ const { mockReadEnvFile: vi.fn<() => Record>(), mockGetActiveCodexAuthPath: vi.fn<() => string | null>(), mockGetCodexAccountCount: vi.fn<() => number>(), - mockClaimCodexAuthLease: vi.fn<() => null>(), + mockClaimCodexAuthLease: vi.fn< + () => { + authPath: string; + accountIndex: number; + release: () => void; + } | null + >(), mockFindCodexAccountIndexByAuthPath: vi.fn<() => number | null>(), })); @@ -835,6 +841,37 @@ describe('prepareReadonlySessionEnvironment codex compatibility', () => { fs.rmSync(tempRoot, { recursive: true, force: true }); }); + it('does not claim a Codex auth lease for Claude read-only sessions', () => { + vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true); + mockGetCodexAccountCount.mockReturnValue(1); + const rotatedAuthPath = path.join(tempRoot, 'rotated-auth.json'); + fs.writeFileSync(rotatedAuthPath, '{"auth_mode":"chatgpt"}\n'); + const release = vi.fn(); + mockClaimCodexAuthLease.mockReturnValue({ + authPath: rotatedAuthPath, + accountIndex: 0, + release, + }); + + const sessionDir = path.join(tempRoot, 'readonly-claude-session'); + const prepared = prepareReadonlySessionEnvironment({ + sessionDir, + chatJid: 'dc:test', + isMain: false, + groupFolder: 'codex-test-group', + agentType: 'claude-code', + memoryBriefing: 'memory briefing', + role: 'reviewer', + }); + + expect(prepared.codexSessionAuth).toBeNull(); + expect(mockClaimCodexAuthLease).not.toHaveBeenCalled(); + expect(release).not.toHaveBeenCalled(); + expect(fs.existsSync(path.join(sessionDir, '.codex', 'auth.json'))).toBe( + false, + ); + }); + it('writes matching AGENTS.md and copies host codex auth/config into the role-scoped session', () => { vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true); diff --git a/src/agent-runner-environment.ts b/src/agent-runner-environment.ts index 0422071..32e2683 100644 --- a/src/agent-runner-environment.ts +++ b/src/agent-runner-environment.ts @@ -832,32 +832,39 @@ export function prepareReadonlySessionEnvironment(args: { const sessionClaudeMdPath = path.join(sessionDir, 'CLAUDE.md'); if (sessionClaudeMd) { fs.writeFileSync(sessionClaudeMdPath, sessionClaudeMd + '\n'); - const sessionCodexDir = path.join(sessionDir, '.codex'); - codexSessionAuth = syncHostCodexSessionFiles(sessionCodexDir); - fs.writeFileSync( - path.join(sessionCodexDir, 'AGENTS.md'), - sessionClaudeMd + '\n', - ); - const sessionConfigPath = path.join(sessionCodexDir, 'config.toml'); - const mcpServerPath = path.join( - projectRoot, - 'runners', - 'agent-runner', - 'dist', - 'ipc-mcp-stdio.js', - ); - if (fs.existsSync(mcpServerPath)) { - upsertEjclawMcpServerSection({ - sessionConfigPath, - mcpServerPath, - ipcDir, - hostIpcDir, - chatJid, - groupFolder, - isMain, - agentType, - roomRole: role, - workDir, + if (agentType === 'codex') { + const sessionCodexDir = path.join(sessionDir, '.codex'); + codexSessionAuth = syncHostCodexSessionFiles(sessionCodexDir); + fs.writeFileSync( + path.join(sessionCodexDir, 'AGENTS.md'), + sessionClaudeMd + '\n', + ); + const sessionConfigPath = path.join(sessionCodexDir, 'config.toml'); + const mcpServerPath = path.join( + projectRoot, + 'runners', + 'agent-runner', + 'dist', + 'ipc-mcp-stdio.js', + ); + if (fs.existsSync(mcpServerPath)) { + upsertEjclawMcpServerSection({ + sessionConfigPath, + mcpServerPath, + ipcDir, + hostIpcDir, + chatJid, + groupFolder, + isMain, + agentType, + roomRole: role, + workDir, + }); + } + } else { + fs.rmSync(path.join(sessionDir, '.codex'), { + recursive: true, + force: true, }); } logger.info( diff --git a/src/agent-runner-process.ts b/src/agent-runner-process.ts index e2e5ec3..bd659d8 100644 --- a/src/agent-runner-process.ts +++ b/src/agent-runner-process.ts @@ -60,6 +60,44 @@ function logStreamedOutputDeliveryError( ); } +function logStreamedAgentErrorOutput( + output: AgentOutput, + group: RegisteredGroup, + input: AgentInput, +): void { + if (output.status !== 'error') return; + logger.warn( + { + group: group.name, + chatJid: input.chatJid, + runId: input.runId, + error: output.error, + newSessionId: output.newSessionId, + }, + 'Streamed agent error output', + ); +} + +function chainStreamedOutputDelivery(args: { + outputChain: Promise; + parsed: AgentOutput; + onOutput: (output: AgentOutput) => Promise; + onTerminalStreamedOutputFlushed?: (output: AgentOutput) => void; + group: RegisteredGroup; + input: AgentInput; +}): Promise { + return args.outputChain.then(async () => { + try { + await args.onOutput(args.parsed); + if (isTerminalStreamedOutput(args.parsed)) { + args.onTerminalStreamedOutputFlushed?.(args.parsed); + } + } catch (err) { + logStreamedOutputDeliveryError(err, args.group, args.input); + } + }); +} + function writeTimeoutLog(args: { logsDir: string; input: AgentInput; @@ -188,27 +226,15 @@ export function runSpawnedAgentProcess( } hadStreamingOutput = true; resetTimeout(); - if (parsed.status === 'error') { - logger.warn( - { - group: group.name, - chatJid: input.chatJid, - runId: input.runId, - error: parsed.error, - newSessionId: parsed.newSessionId, - }, - 'Streamed agent error output', - ); - } - outputChain = outputChain.then(async () => { - try { - await onOutput(parsed); - if (isTerminalStreamedOutput(parsed)) { - args.onTerminalStreamedOutputFlushed?.(parsed); - } - } catch (err) { - logStreamedOutputDeliveryError(err, group, input); - } + logStreamedAgentErrorOutput(parsed, group, input); + outputChain = chainStreamedOutputDelivery({ + outputChain, + parsed, + onOutput, + onTerminalStreamedOutputFlushed: + args.onTerminalStreamedOutputFlushed, + group, + input, }); } catch (err) { logger.warn( diff --git a/src/agent-runner.test.ts b/src/agent-runner.test.ts index cf826e8..9ab63c3 100644 --- a/src/agent-runner.test.ts +++ b/src/agent-runner.test.ts @@ -137,6 +137,72 @@ function emitOutputMarker( proc.stdout.push(`${OUTPUT_START_MARKER}\n${json}\n${OUTPUT_END_MARKER}\n`); } +function mockCodexLeaseEnvironment(releaseLease: () => void) { + return vi + .spyOn(agentRunnerEnvironment, 'prepareGroupEnvironment') + .mockReturnValueOnce({ + env: { + EJCLAW_IPC_DIR: '/tmp/ejclaw-test-data/ipc/test-group', + }, + groupDir: '/tmp/ejclaw-test-groups/test-group', + runnerDir: '/tmp/ejclaw-test-runners/codex-runner', + codexSessionAuth: { + canonicalAuthPath: '/tmp/codex-account/auth.json', + sessionAuthPath: '/tmp/codex-session/auth.json', + accountIndex: 0, + lease: { + accountIndex: 0, + authPath: '/tmp/codex-account/auth.json', + release: releaseLease, + }, + }, + }); +} + +async function expectCodexLeaseReleasedAfterFinalOutputFlush() { + const releaseLease = vi.fn(); + const prepareGroupEnvironmentSpy = mockCodexLeaseEnvironment(releaseLease); + const onOutput = vi.fn(async () => {}); + const codexGroup: RegisteredGroup = { + ...testGroup, + agentType: 'codex', + }; + let resultPromise: Promise | undefined; + + try { + resultPromise = runAgentProcess( + codexGroup, + { + ...testInput, + runId: 'run-codex-lease-final', + }, + () => {}, + onOutput, + ); + + emitOutputMarker(fakeProc, { + status: 'success', + result: 'TASK_DONE\n작업 완료', + phase: 'final', + newSessionId: 'codex-session-lease-final', + }); + await vi.advanceTimersByTimeAsync(10); + + expect(onOutput).toHaveBeenCalledWith( + expect.objectContaining({ + result: 'TASK_DONE\n작업 완료', + phase: 'final', + }), + ); + expect(releaseLease).toHaveBeenCalledTimes(1); + } finally { + fakeProc.emit('close', 0); + await vi.advanceTimersByTimeAsync(10); + await resultPromise?.catch(() => undefined); + prepareGroupEnvironmentSpy.mockRestore(); + } +} + describe('agent-runner timeout behavior', () => { beforeEach(() => { vi.useFakeTimers(); @@ -157,23 +223,18 @@ describe('agent-runner timeout behavior', () => { onOutput, ); - // Emit output with a result emitOutputMarker(fakeProc, { status: 'success', result: 'Here is my response', newSessionId: 'session-123', }); - // Let output processing settle await vi.advanceTimersByTimeAsync(10); - // Fire the hard timeout (IDLE_TIMEOUT + 30s = 1830000ms) await vi.advanceTimersByTimeAsync(1830000); - // Emit close event (as if agent was stopped by the timeout) fakeProc.emit('close', 137); - // Let the promise resolve await vi.advanceTimersByTimeAsync(10); const result = await resultPromise; @@ -193,10 +254,8 @@ describe('agent-runner timeout behavior', () => { onOutput, ); - // No output emitted — fire the hard timeout await vi.advanceTimersByTimeAsync(1830000); - // Emit close event fakeProc.emit('close', 137); await vi.advanceTimersByTimeAsync(10); @@ -216,7 +275,6 @@ describe('agent-runner timeout behavior', () => { onOutput, ); - // Emit output emitOutputMarker(fakeProc, { status: 'success', result: 'Done', @@ -225,7 +283,6 @@ describe('agent-runner timeout behavior', () => { await vi.advanceTimersByTimeAsync(10); - // Normal exit (no timeout) fakeProc.emit('close', 0); await vi.advanceTimersByTimeAsync(10); @@ -236,65 +293,7 @@ describe('agent-runner timeout behavior', () => { }); it('releases Codex auth lease after final streamed output flushes even before process close', async () => { - const releaseLease = vi.fn(); - const prepareGroupEnvironmentSpy = vi - .spyOn(agentRunnerEnvironment, 'prepareGroupEnvironment') - .mockReturnValueOnce({ - env: { - EJCLAW_IPC_DIR: '/tmp/ejclaw-test-data/ipc/test-group', - }, - groupDir: '/tmp/ejclaw-test-groups/test-group', - runnerDir: '/tmp/ejclaw-test-runners/codex-runner', - codexSessionAuth: { - canonicalAuthPath: '/tmp/codex-account/auth.json', - sessionAuthPath: '/tmp/codex-session/auth.json', - accountIndex: 0, - lease: { - accountIndex: 0, - authPath: '/tmp/codex-account/auth.json', - release: releaseLease, - }, - }, - }); - const onOutput = vi.fn(async () => {}); - const codexGroup: RegisteredGroup = { - ...testGroup, - agentType: 'codex', - }; - let resultPromise: Promise | undefined; - - try { - resultPromise = runAgentProcess( - codexGroup, - { - ...testInput, - runId: 'run-codex-lease-final', - }, - () => {}, - onOutput, - ); - - emitOutputMarker(fakeProc, { - status: 'success', - result: 'TASK_DONE\n작업 완료', - phase: 'final', - newSessionId: 'codex-session-lease-final', - }); - await vi.advanceTimersByTimeAsync(10); - - expect(onOutput).toHaveBeenCalledWith( - expect.objectContaining({ - result: 'TASK_DONE\n작업 완료', - phase: 'final', - }), - ); - expect(releaseLease).toHaveBeenCalledTimes(1); - } finally { - fakeProc.emit('close', 0); - await vi.advanceTimersByTimeAsync(10); - await resultPromise?.catch(() => undefined); - prepareGroupEnvironmentSpy.mockRestore(); - } + await expectCodexLeaseReleasedAfterFinalOutputFlush(); }); it('preserves streamed progress phase metadata', async () => { diff --git a/src/message-agent-executor-paired.ts b/src/message-agent-executor-paired.ts index d4fe420..018340b 100644 --- a/src/message-agent-executor-paired.ts +++ b/src/message-agent-executor-paired.ts @@ -623,6 +623,7 @@ class PairedExecutionLifecycleController implements PairedExecutionLifecycle { sawOutput: state.sawOutputForFollowUp, taskStatus: finishedTask?.status ?? null, outputSummary: this.pairedExecutionSummary, + ownerFailureCount: finishedTask?.owner_failure_count ?? null, }); } diff --git a/src/message-agent-executor-rules.test.ts b/src/message-agent-executor-rules.test.ts index 84db5ec..c5ceba1 100644 --- a/src/message-agent-executor-rules.test.ts +++ b/src/message-agent-executor-rules.test.ts @@ -172,17 +172,17 @@ describe('message-agent-executor-rules', () => { ).toBe('pending'); }); - it('does not requeue a silent arbiter failure caused by Codex account auth expiry', () => { + it('requeues owner follow-up after a silent arbiter Codex account failure returns the task active', () => { expect( resolvePairedFollowUpQueueAction({ completedRole: 'arbiter', executionStatus: 'failed', sawOutput: false, - taskStatus: 'arbiter_requested', + taskStatus: 'active', 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'); + ).toBe('pending'); }); it('resolves pending arbiter follow-up requeue after repeated owner execution failures', () => { @@ -207,7 +207,7 @@ describe('message-agent-executor-rules', () => { ).toBe('pending'); }); - it('does not requeue a silent owner failure caused by Codex pool unavailability', () => { + it('requeues a silent owner failure caused by Codex pool unavailability', () => { expect( resolvePairedFollowUpQueueAction({ completedRole: 'owner', @@ -217,7 +217,7 @@ describe('message-agent-executor-rules', () => { outputSummary: 'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex\nExecution completed without a visible terminal verdict.', }), - ).toBe('none'); + ).toBe('pending'); }); it('does not request an executor-side follow-up after successful owner output moved the task to review_ready', () => { diff --git a/src/message-agent-executor-rules.ts b/src/message-agent-executor-rules.ts index 4af85ad..675bbd9 100644 --- a/src/message-agent-executor-rules.ts +++ b/src/message-agent-executor-rules.ts @@ -28,13 +28,19 @@ export function resolvePairedFollowUpQueueAction(args: { sawOutput: boolean; taskStatus: PairedTaskStatus | null; outputSummary?: string | null; + ownerFailureCount?: number | null; }): PairedFollowUpQueueAction { if ( args.executionStatus === 'failed' && args.sawOutput === false && isSilentCodexAccountFailure(args.outputSummary) ) { - return 'none'; + if (args.completedRole === 'arbiter' && args.taskStatus === 'active') { + return 'pending'; + } + if (args.completedRole !== 'owner') { + return 'none'; + } } if ( diff --git a/src/message-runtime-follow-up.ts b/src/message-runtime-follow-up.ts index 8c7c10b..5e9ef4e 100644 --- a/src/message-runtime-follow-up.ts +++ b/src/message-runtime-follow-up.ts @@ -21,6 +21,11 @@ export type PairedFollowUpSource = Parameters< typeof resolveFollowUpDispatch >[0]['source']; +type PairedFollowUpTaskContext = Pick< + PairedTask, + 'id' | 'status' | 'round_trip_count' | 'updated_at' | 'owner_failure_count' +>; + export interface PairedFollowUpDecision { taskId: string | null; taskStatus: PairedTaskStatus | null; @@ -67,7 +72,10 @@ export function resolveLatestPairedTurnOutputContext(args: { } export function resolvePairedFollowUpDecision(args: { - task: Pick | null | undefined; + task: + | Pick + | null + | undefined; source: PairedFollowUpSource; completedRole?: PairedRoomRole; executionStatus?: 'succeeded' | 'failed'; @@ -93,6 +101,7 @@ export function resolvePairedFollowUpDecision(args: { taskStatus: args.task?.status ?? null, lastTurnOutputRole, lastTurnOutputVerdict, + ownerFailureCount: args.task?.owner_failure_count ?? null, }); const dispatch = resolveFollowUpDispatch({ source: args.source, @@ -115,10 +124,7 @@ export function resolvePairedFollowUpDecision(args: { export function schedulePairedFollowUpIntent(args: { chatJid: string; runId: string; - task: - | Pick - | null - | undefined; + task: PairedFollowUpTaskContext | null | undefined; intentKind: ScheduledPairedFollowUpIntentKind; enqueue: () => void; fallbackLastTurnOutputRole?: PairedRoomRole | null; @@ -148,6 +154,7 @@ export function schedulePairedFollowUpIntent(args: { taskStatus: args.task.status, lastTurnOutputRole: latestOutputContext.role, lastTurnOutputVerdict: latestOutputContext.verdict, + ownerFailureCount: args.task.owner_failure_count ?? null, intentKind: args.intentKind, }) && !( @@ -171,10 +178,7 @@ export function schedulePairedFollowUpIntent(args: { export function schedulePairedFollowUpWithMessageCheck(args: { chatJid: string; runId: string; - task: - | Pick - | null - | undefined; + task: PairedFollowUpTaskContext | null | undefined; intentKind: ScheduledPairedFollowUpIntentKind; enqueueMessageCheck: () => void; fallbackLastTurnOutputRole?: PairedRoomRole | null; @@ -198,10 +202,7 @@ export function schedulePairedFollowUpWithMessageCheck(args: { export function dispatchPairedFollowUpForEvent(args: { chatJid: string; runId: string; - task: - | Pick - | null - | undefined; + task: PairedFollowUpTaskContext | null | undefined; source: PairedFollowUpSource; completedRole?: PairedRoomRole; executionStatus?: 'succeeded' | 'failed'; @@ -270,10 +271,7 @@ export function dispatchPairedFollowUpForEvent(args: { export function enqueuePairedFollowUpAfterEvent(args: { chatJid: string; runId: string; - task: - | Pick - | null - | undefined; + task: PairedFollowUpTaskContext | null | undefined; source: PairedFollowUpSource; completedRole?: PairedRoomRole; executionStatus?: 'succeeded' | 'failed'; diff --git a/src/owner-final-codex-unavailable.test.ts b/src/owner-final-codex-unavailable.test.ts new file mode 100644 index 0000000..0856f09 --- /dev/null +++ b/src/owner-final-codex-unavailable.test.ts @@ -0,0 +1,179 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('./db.js', () => { + const updatePairedTask = vi.fn(); + return { + getPairedTaskById: vi.fn(), + getPairedWorkspace: vi.fn(), + updatePairedTask, + updatePairedTaskIfUnchanged: vi.fn((id, _expectedUpdatedAt, updates) => { + updatePairedTask(id, updates); + return true; + }), + releasePairedTaskExecutionLease: vi.fn(), + }; +}); + +vi.mock('./paired-workspace-manager.js', () => ({ + isOwnerWorkspaceRepairNeededError: vi.fn(() => false), + markPairedTaskReviewReady: vi.fn(), + prepareReviewerWorkspaceForExecution: vi.fn(), + provisionOwnerWorkspaceForPairedTask: vi.fn(), +})); + +vi.mock('./logger.js', () => ({ + logger: { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +import * as config from './config.js'; +import * as db from './db.js'; +import { completePairedExecutionContext } from './paired-execution-context.js'; +import { resolvePairedFollowUpQueueAction } from './message-agent-executor-rules.js'; +import type { PairedTask } from './types.js'; + +const CODEX_UNAVAILABLE_SUMMARY = + 'auth-expired: All Codex rotation accounts unavailable; re-auth required before launching Codex\nExecution completed without a visible terminal verdict.'; + +function buildPairedTask(overrides: Partial = {}): PairedTask { + return { + id: 'task-1', + chat_jid: 'dc:test', + group_folder: 'ejclaw', + owner_service_id: config.CODEX_MAIN_SERVICE_ID, + reviewer_service_id: config.REVIEWER_SERVICE_ID_FOR_TYPE, + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: null, + round_trip_count: 0, + owner_failure_count: 0, + owner_step_done_streak: 0, + finalize_step_done_count: 0, + task_done_then_user_reopen_count: 0, + empty_step_done_streak: 0, + status: 'merge_ready', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:00:00.000Z', + ...overrides, + }; +} + +describe('owner final Codex unavailable handling', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(db.getPairedWorkspace).mockReturnValue(undefined); + vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true); + }); + + it('preserves active tasks when the first owner turn cannot start Codex', () => { + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'active', + updated_at: '2026-03-28T00:00:05.000Z', + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'owner', + status: 'failed', + summary: CODEX_UNAVAILABLE_SUMMARY, + }); + + const updates = vi.mocked(db.updatePairedTask).mock.calls[0]?.[1]; + expect(updates).toEqual( + expect.objectContaining({ + owner_failure_count: 1, + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + }), + ); + expect(updates?.status).not.toBe('completed'); + }); + + it('preserves merge_ready when owner finalization cannot start Codex', () => { + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + updated_at: '2026-03-28T00:00:05.000Z', + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'owner', + status: 'failed', + summary: CODEX_UNAVAILABLE_SUMMARY, + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + owner_failure_count: 1, + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + }), + ); + }); + + it('requests arbiter after repeated owner Codex account failures', () => { + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'active', + owner_failure_count: 1, + updated_at: '2026-03-28T00:00:05.000Z', + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'owner', + status: 'failed', + summary: CODEX_UNAVAILABLE_SUMMARY, + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + status: 'arbiter_requested', + owner_failure_count: 2, + arbiter_requested_at: expect.any(String), + }), + ); + }); + + it('requeues owner finalization once after preserving merge_ready', () => { + expect( + resolvePairedFollowUpQueueAction({ + completedRole: 'owner', + executionStatus: 'failed', + sawOutput: false, + taskStatus: 'merge_ready', + ownerFailureCount: 1, + outputSummary: CODEX_UNAVAILABLE_SUMMARY, + }), + ).toBe('pending'); + }); + + it('queues arbiter after repeated owner finalization failures', () => { + expect( + resolvePairedFollowUpQueueAction({ + completedRole: 'owner', + executionStatus: 'failed', + sawOutput: false, + taskStatus: 'arbiter_requested', + ownerFailureCount: 2, + outputSummary: CODEX_UNAVAILABLE_SUMMARY, + }), + ).toBe('pending'); + }); +}); diff --git a/src/paired-arbiter-codex-owner-wake.test.ts b/src/paired-arbiter-codex-owner-wake.test.ts new file mode 100644 index 0000000..ce589ea --- /dev/null +++ b/src/paired-arbiter-codex-owner-wake.test.ts @@ -0,0 +1,81 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('./db.js', () => ({ + getPairedTurnOutputs: vi.fn(() => []), + reservePairedTurnReservation: vi.fn(() => true), +})); + +import { getPairedTurnOutputs } from './db.js'; +import { dispatchPairedFollowUpForEvent } from './message-runtime-follow-up.js'; +import { + matchesExpectedPairedFollowUpIntent, + resolveNextTurnAction, +} from './message-runtime-rules.js'; + +describe('arbiter Codex failure owner wake', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getPairedTurnOutputs).mockReturnValue([]); + }); + + it('keeps owner follow-ups schedulable when active task has owner failure count', () => { + const followUpContext = { + taskStatus: 'active' as const, + lastTurnOutputRole: 'owner' as const, + lastTurnOutputVerdict: 'step_done' as const, + ownerFailureCount: 1, + }; + + expect(resolveNextTurnAction(followUpContext)).toEqual({ + kind: 'owner-follow-up', + }); + expect( + matchesExpectedPairedFollowUpIntent({ + ...followUpContext, + intentKind: 'owner-follow-up', + }), + ).toBe(true); + }); + + it('requeues owner follow-up when arbiter Codex failure returns the task active', () => { + const enqueue = vi.fn(); + vi.mocked(getPairedTurnOutputs).mockReturnValue([ + { + id: 1, + task_id: 'task-arbiter-codex-unavailable', + turn_number: 1, + role: 'owner', + output_text: 'STEP_DONE\nwaiting for CI', + verdict: 'step_done', + created_at: '2026-03-30T00:00:01.000Z', + }, + ] as any); + + const result = dispatchPairedFollowUpForEvent({ + chatJid: 'group@test', + runId: 'run-arbiter-codex-unavailable', + task: { + id: 'task-arbiter-codex-unavailable', + status: 'active', + round_trip_count: 1, + owner_failure_count: 1, + updated_at: '2026-03-30T00:00:02.000Z', + }, + source: 'executor-recovery', + completedRole: 'arbiter', + executionStatus: 'failed', + sawOutput: false, + enqueue, + }); + + expect(result).toMatchObject({ + kind: 'paired-follow-up', + intentKind: 'owner-follow-up', + scheduled: true, + taskStatus: 'active', + lastTurnOutputRole: 'owner', + nextTurnAction: { kind: 'owner-follow-up' }, + }); + expect(enqueue).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/paired-execution-context-arbiter.ts b/src/paired-execution-context-arbiter.ts index 95541a1..7308063 100644 --- a/src/paired-execution-context-arbiter.ts +++ b/src/paired-execution-context-arbiter.ts @@ -14,16 +14,23 @@ export function handleFailedArbiterExecution(args: { const { task, taskId, summary } = args; if (isTerminalCodexAccountFailure(summary)) { const now = new Date().toISOString(); + const nextOwnerFailureCount = Math.max( + (task.owner_failure_count ?? 0) + 1, + 1, + ); transitionPairedTaskStatus({ taskId, currentStatus: task.status, - nextStatus: 'completed', + nextStatus: 'active', expectedUpdatedAt: task.updated_at, updatedAt: now, patch: { - arbiter_verdict: 'escalate', + owner_failure_count: nextOwnerFailureCount, + owner_step_done_streak: 0, + finalize_step_done_count: 0, + empty_step_done_streak: 0, + arbiter_verdict: null, arbiter_requested_at: null, - completion_reason: 'arbiter_codex_unavailable', }, }); logger.warn( @@ -31,9 +38,10 @@ export function handleFailedArbiterExecution(args: { taskId, role: 'arbiter', status: task.status, + nextOwnerFailureCount, summary: summary?.slice(0, 200), }, - 'Completed arbiter task after terminal Codex account failure instead of re-arming arbitration loop', + 'Returned arbiter task to owner after terminal Codex account failure', ); return; } diff --git a/src/paired-execution-context-owner.ts b/src/paired-execution-context-owner.ts index 0ccc06b..80aca0c 100644 --- a/src/paired-execution-context-owner.ts +++ b/src/paired-execution-context-owner.ts @@ -31,33 +31,81 @@ export function handleFailedOwnerExecution(args: { }): void { const { task, taskId, summary } = args; const now = new Date().toISOString(); + const nextFailureCount = (task.owner_failure_count ?? 0) + 1; if (isTerminalCodexAccountFailure(summary)) { + if (nextFailureCount >= OWNER_FAILURE_ESCALATION_THRESHOLD) { + requestArbiterOrEscalate({ + taskId, + currentStatus: task.status, + expectedUpdatedAt: task.updated_at, + now, + arbiterLogMessage: + 'Owner Codex unavailable repeatedly — requesting arbiter', + escalateLogMessage: + 'Owner Codex unavailable repeatedly — escalating to user', + logContext: { + taskId, + role: 'owner', + previousStatus: task.status, + ownerFailureCount: nextFailureCount, + summary: summary?.slice(0, 160), + }, + patch: { + owner_failure_count: nextFailureCount, + arbiter_verdict: null, + completion_reason: null, + }, + }); + return; + } + + const patch = { + owner_failure_count: nextFailureCount, + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + }; + if (task.status === 'active' || task.status === 'merge_ready') { + applyPairedTaskPatch({ + taskId, + expectedUpdatedAt: task.updated_at, + updatedAt: now, + patch, + }); + logger.warn( + { + taskId, + role: 'owner', + status: task.status, + ownerFailureCount: nextFailureCount, + summary: summary?.slice(0, 200), + }, + 'Preserved owner task after terminal Codex account failure', + ); + return; + } + transitionPairedTaskStatus({ taskId, currentStatus: task.status, - nextStatus: 'completed', + nextStatus: 'active', expectedUpdatedAt: task.updated_at, updatedAt: now, - patch: { - arbiter_verdict: 'escalate', - arbiter_requested_at: null, - completion_reason: 'owner_codex_unavailable', - }, + patch, }); logger.warn( { taskId, role: 'owner', status: task.status, + ownerFailureCount: nextFailureCount, summary: summary?.slice(0, 200), }, - 'Completed owner task after terminal Codex account failure instead of retrying owner loop', + 'Reset owner task to active after terminal Codex account failure', ); return; } - const nextFailureCount = (task.owner_failure_count ?? 0) + 1; - if (nextFailureCount >= OWNER_FAILURE_ESCALATION_THRESHOLD) { requestArbiterOrEscalate({ taskId, diff --git a/src/paired-execution-context-routing.test.ts b/src/paired-execution-context-routing.test.ts index 2b80ed5..c0575e7 100644 --- a/src/paired-execution-context-routing.test.ts +++ b/src/paired-execution-context-routing.test.ts @@ -197,10 +197,16 @@ describe('paired execution routing loop guards', () => { ); }); - it('does not re-arm arbiter loop after terminal Codex account failure', () => { + it('returns arbiter terminal Codex account failures to owner without re-arming arbiter', () => { vi.mocked(db.getPairedTaskById).mockReturnValue( buildPairedTask({ status: 'in_arbitration', + owner_failure_count: 1, + owner_step_done_streak: 3, + finalize_step_done_count: 1, + empty_step_done_streak: 2, + arbiter_verdict: 'revise', + arbiter_requested_at: '2026-03-28T00:00:04.000Z', updated_at: '2026-03-28T00:00:05.000Z', }), ); @@ -216,10 +222,13 @@ describe('paired execution routing loop guards', () => { expect(db.updatePairedTask).toHaveBeenCalledWith( 'task-1', expect.objectContaining({ - status: 'completed', - arbiter_verdict: 'escalate', + status: 'active', + owner_failure_count: 2, + owner_step_done_streak: 0, + finalize_step_done_count: 0, + empty_step_done_streak: 0, + arbiter_verdict: null, arbiter_requested_at: null, - completion_reason: 'arbiter_codex_unavailable', }), ); }); @@ -251,7 +260,7 @@ describe('paired execution routing loop guards', () => { ); }); - it('completes owner task after terminal Codex account failure instead of retrying owner forever', () => { + it('preserves owner task after terminal Codex account failure instead of completing silently', () => { vi.mocked(db.getPairedTaskById).mockReturnValue( buildPairedTask({ status: 'active', @@ -270,10 +279,10 @@ describe('paired execution routing loop guards', () => { expect(db.updatePairedTask).toHaveBeenCalledWith( 'task-1', expect.objectContaining({ - status: 'completed', - arbiter_verdict: 'escalate', + owner_failure_count: 1, + arbiter_verdict: null, arbiter_requested_at: null, - completion_reason: 'owner_codex_unavailable', + completion_reason: null, }), ); });