From 90bc154dcf36c6d8cb2df37bd3ab63e071dc8e0d Mon Sep 17 00:00:00 2001 From: ejclaw Date: Tue, 7 Apr 2026 17:55:50 +0900 Subject: [PATCH] Fix paired owner completion handoff race --- src/message-agent-executor.test.ts | 82 ++++++++++++++++++++++++++++++ src/message-agent-executor.ts | 27 +++++++++- src/message-runtime.test.ts | 53 +++++++++++++++++++ 3 files changed, 161 insertions(+), 1 deletion(-) diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 9ff75c8..e2c3921 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -1314,6 +1314,88 @@ describe('runAgentForGroup room memory', () => { expect(deps.queue.enqueueMessageCheck).not.toHaveBeenCalled(); }); + it('stores owner turn output and completes the paired task before delivery', async () => { + const group = { ...makeGroup(), folder: 'test-group' }; + const deps = makeDeps(); + const onOutput = vi.fn(async () => {}); + + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-owner-output-order', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: null, + status: 'active', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-31T00:00:00.000Z', + updated_at: '2026-03-31T00:00:00.000Z', + }, + workspace: null, + envOverrides: {}, + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, forwardOutput) => { + await forwardOutput?.({ + status: 'success', + result: 'DONE_WITH_CONCERNS\nowner complete', + output: { + visibility: 'public', + text: 'DONE_WITH_CONCERNS\nowner complete', + }, + phase: 'final', + }); + return { + status: 'success', + result: 'DONE_WITH_CONCERNS\nowner complete', + }; + }, + ); + + const result = await runAgentForGroup(deps, { + group, + prompt: 'please implement', + chatJid: 'group@test', + runId: 'run-owner-output-order', + onOutput, + }); + + expect(result).toBe('success'); + expect(db.insertPairedTurnOutput).toHaveBeenCalledWith( + 'paired-task-owner-output-order', + 1, + 'owner', + 'DONE_WITH_CONCERNS\nowner complete', + ); + expect( + vi.mocked(db.insertPairedTurnOutput).mock.invocationCallOrder[0], + ).toBeLessThan( + vi.mocked(pairedExecutionContext.completePairedExecutionContext).mock + .invocationCallOrder[0], + ); + expect( + vi.mocked(pairedExecutionContext.completePairedExecutionContext).mock + .invocationCallOrder[0], + ).toBeLessThan(onOutput.mock.invocationCallOrder[0]); + expect( + vi.mocked(pairedExecutionContext.completePairedExecutionContext), + ).toHaveBeenCalledWith({ + taskId: 'paired-task-owner-output-order', + role: 'owner', + status: 'succeeded', + summary: 'DONE_WITH_CONCERNS\nowner complete', + }); + }); + it('stores reviewer turn output before transitioning the paired task back to active', async () => { const group = { ...makeGroup(), folder: 'test-group' }; const deps = makeDeps(); diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 039ad01..65ca577 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -337,6 +337,30 @@ export async function runAgentForGroup( pairedTurnOutputPersisted = true; }; + const completeSuccessfulOwnerTurnBeforeDeliveryIfNeeded = () => { + const completedRole = roomRoleContext?.role ?? 'owner'; + if ( + completedRole !== 'owner' || + !pairedExecutionContext || + pairedExecutionCompleted || + !pairedFinalOutput || + pairedFinalOutput.length === 0 + ) { + return; + } + + pairedExecutionStatus = 'succeeded'; + pairedSawOutput = true; + persistPairedTurnOutputIfNeeded(completedRole); + completePairedExecutionContext({ + taskId: pairedExecutionContext.task.id, + role: completedRole, + status: 'succeeded', + summary: pairedExecutionSummary, + }); + pairedExecutionCompleted = true; + }; + const maybeHandoffToCodex = ( reason: AgentTriggerReason, sawVisibleOutput: boolean, @@ -567,11 +591,12 @@ export async function runAgentForGroup( ) { pairedFinalOutput = outputText; try { + completeSuccessfulOwnerTurnBeforeDeliveryIfNeeded(); persistPairedTurnOutputIfNeeded(roomRoleContext?.role ?? 'owner'); } catch (err) { log.warn( { pairedTaskId: pairedExecutionContext?.task.id ?? null, err }, - 'Failed to persist paired turn output before delivery', + 'Failed to persist paired turn output and status before delivery', ); } } diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index b05e509..f9df2a3 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -1645,6 +1645,59 @@ describe('createMessageRuntime', () => { }); }); + it('requeues the reviewer follow-up once owner output is persisted under review_ready', () => { + const chatJid = 'group@test'; + const group = makeGroup('codex'); + const task = { + id: 'task-reviewer-follow-up-after-owner-output', + chat_jid: chatJid, + group_folder: group.folder, + owner_service_id: 'claude', + reviewer_service_id: 'codex-main', + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: '2026-03-30T00:00:00.000Z', + round_trip_count: 1, + status: 'review_ready', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-30T00:00:00.000Z', + updated_at: '2026-03-30T00:00:00.000Z', + } as any; + + vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true); + vi.mocked(db.getPairedTurnOutputs).mockReturnValue([ + { + id: 1, + task_id: 'task-reviewer-follow-up-after-owner-output', + turn_number: 1, + role: 'owner', + output_text: 'owner 응답', + created_at: '2026-03-30T00:00:01.000Z', + }, + ]); + + expect( + resolveBotOnlyPairedFollowUpAction({ + chatJid, + task, + isBotOnlyPairedFollowUp: true, + pendingCursorSource: { + seq: 42, + timestamp: '2026-03-30T00:00:04.000Z', + }, + }), + ).toEqual({ + kind: 'requeue-pending-turn', + task, + cursor: 42, + cursorKey: `${chatJid}:reviewer`, + nextRole: 'reviewer', + }); + }); + it('consumes stale bot-only owner messages once the finalize turn output is already persisted', () => { const chatJid = 'group@test'; const group = makeGroup('codex');