From 7ad45869852422c576fc67c4a2d35c3ef0464373 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Tue, 7 Apr 2026 18:39:00 +0900 Subject: [PATCH] Restore reviewer wake-up after owner delivery --- src/message-agent-executor.ts | 5 +- src/message-runtime.test.ts | 105 ++++++++++++++++++++++++++++++++++ src/message-runtime.ts | 29 ++++++++-- 3 files changed, 133 insertions(+), 6 deletions(-) diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 65ca577..8479cd5 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -1017,8 +1017,9 @@ export async function runAgentForGroup( } // Failed reviewer/arbiter runs can leave a pending paired task without a - // visible bot message. Requeue only those recovery cases here; successful - // paired handoffs are driven by the delivered bot message itself. + // visible bot message. Requeue only those recovery cases here. Successful + // owner -> reviewer handoffs are queued by message-runtime after final + // delivery succeeds, which avoids waking reviewer off a failed send. if (pairedExecutionContext) { const completedRole = roomRoleContext?.role ?? 'owner'; const finishedCheck = getPairedTaskById(pairedExecutionContext.task.id); diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index f9df2a3..a3aa5f9 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -1698,6 +1698,111 @@ describe('createMessageRuntime', () => { }); }); + it('re-enqueues reviewer after a successful owner delivery moves the task to review_ready', async () => { + const chatJid = 'group@test'; + const group = makeGroup('codex'); + const ownerChannel: Channel = { + ...makeChannel(chatJid), + isOwnMessage: vi.fn((msg) => msg.sender === 'owner-bot@test'), + }; + const reviewerChannel = makeChannel(chatJid, 'discord-review', false); + const enqueueMessageCheck = vi.fn(); + const pairedTask = { + id: 'task-owner-delivery-follow-up', + 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: null, + round_trip_count: 0, + status: 'active', + 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.getLatestOpenPairedTaskForChat).mockImplementation( + () => pairedTask, + ); + vi.mocked(db.getMessagesSince).mockReturnValue([ + { + id: 'human-1', + chat_jid: chatJid, + sender: 'user@test', + sender_name: 'User', + content: '이 구현 진행해줘', + timestamp: '2026-03-30T00:00:00.000Z', + seq: 1, + is_bot_message: false, + } as any, + ]); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + pairedTask.status = 'review_ready'; + pairedTask.review_requested_at = '2026-03-30T00:00:01.000Z'; + pairedTask.round_trip_count = 1; + await onOutput?.({ + status: 'success', + phase: 'final', + result: 'DONE_WITH_CONCERNS\nowner complete', + newSessionId: 'session-owner-delivery-follow-up', + }); + return { + status: 'success', + result: 'DONE_WITH_CONCERNS\nowner complete', + newSessionId: 'session-owner-delivery-follow-up', + }; + }, + ); + + const runtime = createMessageRuntime({ + assistantName: 'Andy', + idleTimeout: 1_000, + pollInterval: 1_000, + timezone: 'UTC', + triggerPattern: /^@Andy\b/i, + channels: [ownerChannel, reviewerChannel], + queue: { + registerProcess: vi.fn(), + closeStdin: vi.fn(), + notifyIdle: vi.fn(), + enqueueMessageCheck, + } as any, + getRegisteredGroups: () => ({ [chatJid]: group }), + getSessions: () => ({}), + getLastTimestamp: () => '', + setLastTimestamp: vi.fn(), + getLastAgentTimestamps: () => ({}), + saveState: vi.fn(), + persistSession: vi.fn(), + clearSession: vi.fn(), + }); + + const result = await runtime.processGroupMessages(chatJid, { + runId: 'run-owner-delivery-follow-up', + reason: 'messages', + }); + + expect(result).toBe(true); + expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + chatJid, + runId: 'run-owner-delivery-follow-up', + completedRole: 'owner', + taskId: 'task-owner-delivery-follow-up', + taskStatus: 'review_ready', + }), + 'Queued paired follow-up after successful owner delivery', + ); + }); + it('consumes stale bot-only owner messages once the finalize turn output is already persisted', () => { const chatJid = 'group@test'; const group = makeGroup('codex'); diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 05678a7..52eba53 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -403,6 +403,9 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { const isClaudeCodeAgent = (args.forcedAgentType ?? group.agentType ?? 'claude-code') === 'claude-code'; + const pairedRoom = hasReviewerLease(chatJid); + const resolvedDeliveryRole = + args.deliveryRole ?? args.forcedRole ?? (pairedRoom ? 'owner' : null); const turnController = new MessageTurnController({ chatJid, group, @@ -416,10 +419,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { deps.queue.closeStdin(chatJid, { runId, reason }), deliverFinalText: async (text) => { try { - const persistedDeliveryRole = - args.deliveryRole ?? - args.forcedRole ?? - (hasReviewerLease(chatJid) ? 'owner' : null); + const persistedDeliveryRole = resolvedDeliveryRole; if ( (persistedDeliveryRole === 'reviewer' || persistedDeliveryRole === 'arbiter') && @@ -483,6 +483,27 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { const { deliverySucceeded, visiblePhase } = await turnController.finish(outputStatus); + if (deliverySucceeded && pairedRoom && resolvedDeliveryRole === 'owner') { + const pendingTaskAfterDelivery = getLatestOpenPairedTaskForChat(chatJid); + const nextTurnAction = resolveNextTurnAction({ + taskStatus: pendingTaskAfterDelivery?.status, + lastTurnOutputRole: 'owner', + }); + if (nextTurnAction.kind === 'reviewer-turn') { + deps.queue.enqueueMessageCheck(chatJid); + logger.info( + { + chatJid, + runId, + completedRole: resolvedDeliveryRole, + taskId: pendingTaskAfterDelivery?.id ?? null, + taskStatus: pendingTaskAfterDelivery?.status ?? null, + }, + 'Queued paired follow-up after successful owner delivery', + ); + } + } + return { outputStatus, deliverySucceeded,