From 2098db72d2a82fb80c94869f60e13054f0b981b6 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Wed, 29 Apr 2026 11:27:32 +0900 Subject: [PATCH] Extract message runtime after-delivery follow-up --- src/message-runtime-after-delivery.test.ts | 190 +++++++++++++++++++++ src/message-runtime-after-delivery.ts | 68 ++++++++ src/message-runtime.ts | 59 +------ 3 files changed, 265 insertions(+), 52 deletions(-) create mode 100644 src/message-runtime-after-delivery.test.ts create mode 100644 src/message-runtime-after-delivery.ts diff --git a/src/message-runtime-after-delivery.test.ts b/src/message-runtime-after-delivery.test.ts new file mode 100644 index 0000000..b01d0c1 --- /dev/null +++ b/src/message-runtime-after-delivery.test.ts @@ -0,0 +1,190 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('./db.js', () => ({ + getLatestOpenPairedTaskForChat: vi.fn(), + hasActiveCiWatcherForChat: vi.fn(() => false), +})); + +vi.mock('./logger.js', () => ({ + logger: { + info: vi.fn(), + }, +})); + +vi.mock('./message-runtime-follow-up.js', () => ({ + enqueuePairedFollowUpAfterEvent: vi.fn(), +})); + +import { + getLatestOpenPairedTaskForChat, + hasActiveCiWatcherForChat, +} from './db.js'; +import { logger } from './logger.js'; +import { handleMessageRuntimeAfterDeliverySuccess } from './message-runtime-after-delivery.js'; +import { enqueuePairedFollowUpAfterEvent } from './message-runtime-follow-up.js'; +import type { PairedTask } from './types.js'; + +function makeTask(overrides: Partial = {}): PairedTask { + return { + id: 'task-1', + chat_jid: 'chat-1', + group_folder: 'room', + owner_service_id: 'claude', + reviewer_service_id: 'codex-main', + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: null, + round_trip_count: 1, + status: 'active', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-04-29T01:00:00.000Z', + updated_at: '2026-04-29T01:00:00.000Z', + ...overrides, + }; +} + +describe('message-runtime-after-delivery', () => { + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(hasActiveCiWatcherForChat).mockReturnValue(false); + }); + + it('skips non-paired and role-less deliveries without side effects', async () => { + const enqueueMessageCheck = vi.fn(); + + await handleMessageRuntimeAfterDeliverySuccess({ + chatJid: 'chat-1', + runId: 'run-1', + deliveryRole: null, + pairedRoom: true, + enqueueMessageCheck, + }); + await handleMessageRuntimeAfterDeliverySuccess({ + chatJid: 'chat-1', + runId: 'run-2', + deliveryRole: 'owner', + pairedRoom: false, + enqueueMessageCheck, + }); + + expect(getLatestOpenPairedTaskForChat).not.toHaveBeenCalled(); + expect(enqueuePairedFollowUpAfterEvent).not.toHaveBeenCalled(); + expect(enqueueMessageCheck).not.toHaveBeenCalled(); + }); + + it('defers owner follow-up while CI watcher is active after review-ready delivery', async () => { + const task = makeTask({ + id: 'task-review-ready', + status: 'review_ready', + }); + vi.mocked(getLatestOpenPairedTaskForChat).mockReturnValue(task); + vi.mocked(hasActiveCiWatcherForChat).mockReturnValue(true); + const enqueueMessageCheck = vi.fn(); + + await handleMessageRuntimeAfterDeliverySuccess({ + chatJid: 'chat-1', + runId: 'run-ci-watcher', + deliveryRole: 'owner', + pairedRoom: true, + enqueueMessageCheck, + }); + + expect(hasActiveCiWatcherForChat).toHaveBeenCalledWith('chat-1'); + expect(enqueuePairedFollowUpAfterEvent).not.toHaveBeenCalled(); + expect(enqueueMessageCheck).not.toHaveBeenCalled(); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + chatJid: 'chat-1', + runId: 'run-ci-watcher', + completedRole: 'owner', + taskId: 'task-review-ready', + taskStatus: 'review_ready', + }), + 'Deferred paired follow-up after successful owner delivery because CI watcher is still active', + ); + }); + + it('schedules reviewer/arbiter delivery follow-up through message-check enqueue', async () => { + const task = makeTask({ id: 'task-active', status: 'active' }); + vi.mocked(getLatestOpenPairedTaskForChat).mockReturnValue(task); + vi.mocked(enqueuePairedFollowUpAfterEvent).mockImplementation( + (args: any) => { + args.enqueueMessageCheck(); + return { + kind: 'paired-follow-up', + taskId: 'task-active', + taskStatus: 'active', + intentKind: 'owner-follow-up', + scheduled: true, + } as any; + }, + ); + const enqueueMessageCheck = vi.fn(); + + await handleMessageRuntimeAfterDeliverySuccess({ + chatJid: 'chat-1', + runId: 'run-reviewer', + deliveryRole: 'reviewer', + pairedRoom: true, + enqueueMessageCheck, + }); + + expect(enqueuePairedFollowUpAfterEvent).toHaveBeenCalledWith( + expect.objectContaining({ + chatJid: 'chat-1', + runId: 'run-reviewer', + task, + source: 'delivery-success', + completedRole: 'reviewer', + fallbackLastTurnOutputRole: 'reviewer', + }), + ); + expect(enqueueMessageCheck).toHaveBeenCalledWith('chat-1'); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + chatJid: 'chat-1', + runId: 'run-reviewer', + completedRole: 'reviewer', + taskId: 'task-active', + taskStatus: 'active', + intentKind: 'owner-follow-up', + scheduled: true, + }), + 'Queued paired follow-up after successful reviewer/arbiter delivery', + ); + }); + + it('logs duplicate owner follow-up suppression when scheduler returns unscheduled', async () => { + const task = makeTask({ id: 'task-review-ready', status: 'review_ready' }); + vi.mocked(getLatestOpenPairedTaskForChat).mockReturnValue(task); + vi.mocked(enqueuePairedFollowUpAfterEvent).mockReturnValue({ + kind: 'paired-follow-up', + taskId: 'task-review-ready', + taskStatus: 'review_ready', + intentKind: 'reviewer-turn', + scheduled: false, + } as any); + + await handleMessageRuntimeAfterDeliverySuccess({ + chatJid: 'chat-1', + runId: 'run-owner-duplicate', + deliveryRole: 'owner', + pairedRoom: true, + enqueueMessageCheck: vi.fn(), + }); + + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + completedRole: 'owner', + taskId: 'task-review-ready', + taskStatus: 'review_ready', + intentKind: 'reviewer-turn', + scheduled: false, + }), + 'Skipped duplicate paired follow-up after successful owner delivery while task state was unchanged', + ); + }); +}); diff --git a/src/message-runtime-after-delivery.ts b/src/message-runtime-after-delivery.ts new file mode 100644 index 0000000..464c7ea --- /dev/null +++ b/src/message-runtime-after-delivery.ts @@ -0,0 +1,68 @@ +import { + getLatestOpenPairedTaskForChat, + hasActiveCiWatcherForChat, +} from './db.js'; +import { logger } from './logger.js'; +import { enqueuePairedFollowUpAfterEvent } from './message-runtime-follow-up.js'; +import type { PairedRoomRole } from './types.js'; + +export async function handleMessageRuntimeAfterDeliverySuccess(args: { + chatJid: string; + runId: string; + deliveryRole: PairedRoomRole | null; + pairedRoom: boolean; + enqueueMessageCheck: (chatJid: string) => void; +}): Promise { + if (!args.deliveryRole || !args.pairedRoom) { + return; + } + + const pendingTaskAfterDelivery = getLatestOpenPairedTaskForChat(args.chatJid); + if ( + args.deliveryRole === 'owner' && + pendingTaskAfterDelivery?.status === 'review_ready' && + hasActiveCiWatcherForChat(args.chatJid) + ) { + logger.info( + { + chatJid: args.chatJid, + runId: args.runId, + completedRole: args.deliveryRole, + taskId: pendingTaskAfterDelivery.id, + taskStatus: pendingTaskAfterDelivery.status, + }, + 'Deferred paired follow-up after successful owner delivery because CI watcher is still active', + ); + return; + } + + const followUpResult = enqueuePairedFollowUpAfterEvent({ + chatJid: args.chatJid, + runId: args.runId, + task: pendingTaskAfterDelivery, + source: 'delivery-success', + completedRole: args.deliveryRole, + fallbackLastTurnOutputRole: args.deliveryRole, + enqueueMessageCheck: () => args.enqueueMessageCheck(args.chatJid), + }); + if (followUpResult.kind === 'paired-follow-up') { + logger.info( + { + chatJid: args.chatJid, + runId: args.runId, + completedRole: args.deliveryRole, + taskId: followUpResult.taskId, + taskStatus: followUpResult.taskStatus, + intentKind: followUpResult.intentKind, + scheduled: followUpResult.scheduled, + }, + followUpResult.scheduled + ? args.deliveryRole === 'owner' + ? 'Queued paired follow-up after successful owner delivery' + : 'Queued paired follow-up after successful reviewer/arbiter delivery' + : args.deliveryRole === 'owner' + ? 'Skipped duplicate paired follow-up after successful owner delivery while task state was unchanged' + : 'Skipped duplicate paired follow-up after successful reviewer/arbiter delivery while task state was unchanged', + ); + } +} diff --git a/src/message-runtime.ts b/src/message-runtime.ts index a7d29f5..23a5b4a 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -1,6 +1,5 @@ import { createProducedWorkItem, - hasActiveCiWatcherForChat, getOpenWorkItemForChat, getMessagesSinceSeq, getLatestOpenPairedTaskForChat, @@ -43,10 +42,7 @@ import { shouldSkipBotOnlyCollaboration, } from './message-runtime-rules.js'; import { resolveOwnerTaskForHumanMessage } from './paired-execution-context.js'; -import { - enqueuePairedFollowUpAfterEvent, - schedulePairedFollowUpWithMessageCheck, -} from './message-runtime-follow-up.js'; +import { schedulePairedFollowUpWithMessageCheck } from './message-runtime-follow-up.js'; import { type ScheduledPairedFollowUpIntentKind } from './paired-follow-up-scheduler.js'; import { transitionPairedTaskStatus } from './paired-task-status.js'; import { @@ -77,6 +73,7 @@ import { getFreshHumanPreflightMessages, hasHumanMessageAfterWorkItem, } from './message-runtime-preflight-messages.js'; +import { handleMessageRuntimeAfterDeliverySuccess } from './message-runtime-after-delivery.js'; export { isDuplicateOfLastBotFinal }; @@ -243,56 +240,14 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { deliveryRole, pairedRoom, }) => { - if (!deliveryRole || !pairedRoom) { - return; - } - const pendingTaskAfterDelivery = getLatestOpenPairedTaskForChat(chatJid); - if ( - deliveryRole === 'owner' && - pendingTaskAfterDelivery?.status === 'review_ready' && - hasActiveCiWatcherForChat(chatJid) - ) { - logger.info( - { - chatJid, - runId, - completedRole: deliveryRole, - taskId: pendingTaskAfterDelivery.id, - taskStatus: pendingTaskAfterDelivery.status, - }, - 'Deferred paired follow-up after successful owner delivery because CI watcher is still active', - ); - return; - } - const followUpResult = enqueuePairedFollowUpAfterEvent({ + await handleMessageRuntimeAfterDeliverySuccess({ chatJid, runId, - task: pendingTaskAfterDelivery, - source: 'delivery-success', - completedRole: deliveryRole, - fallbackLastTurnOutputRole: deliveryRole, - enqueueMessageCheck: () => deps.queue.enqueueMessageCheck(chatJid), + deliveryRole, + pairedRoom, + enqueueMessageCheck: (targetChatJid) => + deps.queue.enqueueMessageCheck(targetChatJid), }); - if (followUpResult.kind === 'paired-follow-up') { - logger.info( - { - chatJid, - runId, - completedRole: deliveryRole, - taskId: followUpResult.taskId, - taskStatus: followUpResult.taskStatus, - intentKind: followUpResult.intentKind, - scheduled: followUpResult.scheduled, - }, - followUpResult.scheduled - ? deliveryRole === 'owner' - ? 'Queued paired follow-up after successful owner delivery' - : 'Queued paired follow-up after successful reviewer/arbiter delivery' - : deliveryRole === 'owner' - ? 'Skipped duplicate paired follow-up after successful owner delivery while task state was unchanged' - : 'Skipped duplicate paired follow-up after successful reviewer/arbiter delivery while task state was unchanged', - ); - } }, });