diff --git a/src/message-runtime-dispatch.ts b/src/message-runtime-dispatch.ts index 8b46ab0..bc51893 100644 --- a/src/message-runtime-dispatch.ts +++ b/src/message-runtime-dispatch.ts @@ -67,7 +67,7 @@ export function enqueueGenericFollowUpAfterDeliveryRetry(args: { deliveryRole: args.deliveryRole, pendingTaskStatus: args.pendingTask?.status ?? null, }, - 'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff', + 'No queued follow-up was required after delivery retry', ); return; } @@ -90,6 +90,21 @@ export function enqueueGenericFollowUpAfterDeliveryRetry(args: { nextTurnAction.kind, args.runId, ); + if (scheduled) { + args.log.info( + { + workItemId: args.workItemId, + chatJid: args.chatJid, + deliveryRole: args.deliveryRole, + taskId: args.pendingTask?.id ?? null, + pendingTaskStatus: args.pendingTask?.status ?? null, + taskStatus: args.pendingTask?.status ?? null, + intentKind: nextTurnAction.kind, + }, + 'Queued paired follow-up after delivery retry', + ); + return; + } if (!scheduled) { args.log.info( { @@ -97,6 +112,7 @@ export function enqueueGenericFollowUpAfterDeliveryRetry(args: { chatJid: args.chatJid, deliveryRole: args.deliveryRole, taskId: args.pendingTask?.id ?? null, + pendingTaskStatus: args.pendingTask?.status ?? null, taskStatus: args.pendingTask?.status ?? null, intentKind: nextTurnAction.kind, }, diff --git a/src/message-runtime-rules.test.ts b/src/message-runtime-rules.test.ts index 1504062..bd681ee 100644 --- a/src/message-runtime-rules.test.ts +++ b/src/message-runtime-rules.test.ts @@ -105,6 +105,39 @@ describe('message-runtime-rules', () => { ).toEqual({ kind: 'none' }); }); + it('dispatches reviewer and arbiter delivery success through paired follow-up enqueue when a handoff is pending', () => { + expect( + resolveFollowUpDispatch({ + source: 'delivery-success', + nextTurnAction: { kind: 'owner-follow-up' }, + completedRole: 'reviewer', + }), + ).toEqual({ + kind: 'enqueue', + queueKind: 'paired-follow-up', + }); + expect( + resolveFollowUpDispatch({ + source: 'delivery-success', + nextTurnAction: { kind: 'finalize-owner-turn' }, + completedRole: 'reviewer', + }), + ).toEqual({ + kind: 'enqueue', + queueKind: 'paired-follow-up', + }); + expect( + resolveFollowUpDispatch({ + source: 'delivery-success', + nextTurnAction: { kind: 'owner-follow-up' }, + completedRole: 'arbiter', + }), + ).toEqual({ + kind: 'enqueue', + queueKind: 'paired-follow-up', + }); + }); + it('dispatches delivery retry follow-ups through either generic message checks or paired follow-up enqueue', () => { expect( resolveFollowUpDispatch({ @@ -132,7 +165,10 @@ describe('message-runtime-rules', () => { nextTurnAction: { kind: 'finalize-owner-turn' }, completedRole: 'reviewer', }), - ).toEqual({ kind: 'none' }); + ).toEqual({ + kind: 'enqueue', + queueKind: 'paired-follow-up', + }); }); it('dispatches bot-only follow-ups through inline finalize or paired enqueue', () => { diff --git a/src/message-runtime-rules.ts b/src/message-runtime-rules.ts index 9444324..1a495e0 100644 --- a/src/message-runtime-rules.ts +++ b/src/message-runtime-rules.ts @@ -118,6 +118,7 @@ export function resolveNextTurnAction(args: { export function resolveFollowUpDispatch(args: { source: + | 'delivery-success' | 'owner-delivery-success' | 'delivery-retry' | 'bot-only-follow-up' @@ -128,18 +129,21 @@ export function resolveFollowUpDispatch(args: { sawOutput?: boolean; }): FollowUpDispatch { switch (args.source) { + case 'delivery-success': case 'owner-delivery-success': - return args.nextTurnAction.kind === 'reviewer-turn' - ? { kind: 'enqueue', queueKind: 'paired-follow-up' } - : { kind: 'none' }; + if ( + args.source === 'owner-delivery-success' || + args.completedRole === 'owner' + ) { + return args.nextTurnAction.kind === 'reviewer-turn' + ? { kind: 'enqueue', queueKind: 'paired-follow-up' } + : { kind: 'none' }; + } + return args.nextTurnAction.kind === 'none' + ? { kind: 'none' } + : { kind: 'enqueue', queueKind: 'paired-follow-up' }; case 'delivery-retry': - if ( - args.completedRole === 'reviewer' && - args.nextTurnAction.kind === 'finalize-owner-turn' - ) { - return { kind: 'none' }; - } if (args.nextTurnAction.kind === 'none') { return { kind: 'enqueue', queueKind: 'message-check' }; } diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index 277616e..109bf99 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -882,15 +882,17 @@ describe('createMessageRuntime', () => { 'reviewer final retry', ); expect(ownerChannel.sendMessage).not.toHaveBeenCalled(); - expect(enqueueMessageCheck).not.toHaveBeenCalled(); + expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); expect(logger.info).toHaveBeenCalledWith( expect.objectContaining({ workItemId: 101, chatJid, deliveryRole: 'reviewer', + taskId: 'task-review-delivery-role', pendingTaskStatus: 'merge_ready', + intentKind: 'finalize-owner-turn', }), - 'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff', + 'Queued paired follow-up after delivery retry', ); }); @@ -975,15 +977,17 @@ describe('createMessageRuntime', () => { 'fallback reviewer final retry', ); expect(ownerChannel.sendMessage).not.toHaveBeenCalled(); - expect(enqueueMessageCheck).not.toHaveBeenCalled(); + expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); expect(logger.info).toHaveBeenCalledWith( expect.objectContaining({ workItemId: 102, chatJid, deliveryRole: 'reviewer', + taskId: 'task-fallback-review-delivery-role', pendingTaskStatus: 'merge_ready', + intentKind: 'finalize-owner-turn', }), - 'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff', + 'Queued paired follow-up after delivery retry', ); }); @@ -1920,6 +1924,287 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead ); }); + it('re-enqueues owner after a successful reviewer delivery moves the task back to active', async () => { + const chatJid = 'group@test'; + const group = makeGroup('codex'); + const ownerChannel = makeChannel(chatJid); + const reviewerChannel = makeChannel(chatJid, 'discord-review', false); + const enqueueMessageCheck = vi.fn(); + const pairedTask = { + id: 'task-reviewer-delivery-owner-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: '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.getLatestOpenPairedTaskForChat).mockImplementation( + () => pairedTask, + ); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + pairedTask.status = 'active'; + await onOutput?.({ + status: 'success', + phase: 'final', + result: 'DONE_WITH_CONCERNS\nreviewer follow-up needed', + newSessionId: 'session-reviewer-delivery-owner-follow-up', + }); + return { + status: 'success', + result: 'DONE_WITH_CONCERNS\nreviewer follow-up needed', + newSessionId: 'session-reviewer-delivery-owner-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-reviewer-delivery-owner-follow-up', + reason: 'messages', + }); + + expect(result).toBe(true); + expect(reviewerChannel.sendMessage).toHaveBeenCalledWith( + chatJid, + 'DONE_WITH_CONCERNS\nreviewer follow-up needed', + ); + expect(ownerChannel.sendMessage).not.toHaveBeenCalled(); + expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + chatJid, + runId: 'run-reviewer-delivery-owner-follow-up', + completedRole: 'reviewer', + taskId: 'task-reviewer-delivery-owner-follow-up', + taskStatus: 'active', + }), + 'Queued paired follow-up after successful reviewer/arbiter delivery', + ); + }); + + it('re-enqueues finalize-owner after a successful reviewer delivery moves the task to merge_ready', async () => { + const chatJid = 'group@test'; + const group = makeGroup('codex'); + const ownerChannel = makeChannel(chatJid); + const reviewerChannel = makeChannel(chatJid, 'discord-review', false); + const enqueueMessageCheck = vi.fn(); + const pairedTask = { + id: 'task-reviewer-delivery-finalize-owner', + 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.getLatestOpenPairedTaskForChat).mockImplementation( + () => pairedTask, + ); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + pairedTask.status = 'merge_ready'; + await onOutput?.({ + status: 'success', + phase: 'final', + result: 'DONE\nreview approved', + newSessionId: 'session-reviewer-delivery-finalize-owner', + }); + return { + status: 'success', + result: 'DONE\nreview approved', + newSessionId: 'session-reviewer-delivery-finalize-owner', + }; + }, + ); + + 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-reviewer-delivery-finalize-owner', + reason: 'messages', + }); + + expect(result).toBe(true); + expect(reviewerChannel.sendMessage).toHaveBeenCalledWith( + chatJid, + 'DONE\nreview approved', + ); + expect(ownerChannel.sendMessage).not.toHaveBeenCalled(); + expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + chatJid, + runId: 'run-reviewer-delivery-finalize-owner', + completedRole: 'reviewer', + taskId: 'task-reviewer-delivery-finalize-owner', + taskStatus: 'merge_ready', + }), + 'Queued paired follow-up after successful reviewer/arbiter delivery', + ); + }); + + it('re-enqueues owner after a successful arbiter delivery moves the task back to active', async () => { + const chatJid = 'group@test'; + const group = makeGroup('codex'); + const ownerChannel = makeChannel(chatJid); + const reviewerChannel = makeChannel(chatJid, 'discord-review', false); + const arbiterChannel = makeChannel(chatJid, 'discord-arbiter', false); + const enqueueMessageCheck = vi.fn(); + const pairedTask = { + id: 'task-arbiter-delivery-owner-follow-up', + chat_jid: chatJid, + group_folder: group.folder, + owner_service_id: 'claude', + reviewer_service_id: 'codex-main', + arbiter_service_id: 'claude-arbiter', + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: '2026-03-30T00:00:00.000Z', + round_trip_count: 1, + status: 'arbiter_requested', + arbiter_verdict: null, + arbiter_requested_at: '2026-03-30T00:00:10.000Z', + 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(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + pairedTask.status = 'active'; + await onOutput?.({ + status: 'success', + phase: 'final', + result: 'DONE_WITH_CONCERNS\narbiter says revise', + newSessionId: 'session-arbiter-delivery-owner-follow-up', + }); + return { + status: 'success', + result: 'DONE_WITH_CONCERNS\narbiter says revise', + newSessionId: 'session-arbiter-delivery-owner-follow-up', + }; + }, + ); + + const runtime = createMessageRuntime({ + assistantName: 'Andy', + idleTimeout: 1_000, + pollInterval: 1_000, + timezone: 'UTC', + triggerPattern: /^@Andy\b/i, + channels: [ownerChannel, reviewerChannel, arbiterChannel], + 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-arbiter-delivery-owner-follow-up', + reason: 'messages', + }); + + expect(result).toBe(true); + expect(arbiterChannel.sendMessage).toHaveBeenCalledWith( + chatJid, + 'DONE_WITH_CONCERNS\narbiter says revise', + ); + expect(ownerChannel.sendMessage).not.toHaveBeenCalled(); + expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + chatJid, + runId: 'run-arbiter-delivery-owner-follow-up', + completedRole: 'arbiter', + taskId: 'task-arbiter-delivery-owner-follow-up', + taskStatus: 'active', + }), + 'Queued paired follow-up after successful reviewer/arbiter delivery', + ); + }); + it('does not enqueue the same reviewer follow-up twice across different runs while task state is unchanged', async () => { const chatJid = 'group@test'; const group = makeGroup('codex'); diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 8a8d37e..9ee7937 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -343,16 +343,17 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { const { deliverySucceeded, visiblePhase } = await turnController.finish(outputStatus); - if (deliverySucceeded && pairedRoom && resolvedDeliveryRole === 'owner') { + if (deliverySucceeded && pairedRoom && resolvedDeliveryRole) { const pendingTaskAfterDelivery = getLatestOpenPairedTaskForChat(chatJid); const nextTurnAction = resolveNextTurnAction({ taskStatus: pendingTaskAfterDelivery?.status, - lastTurnOutputRole: 'owner', + lastTurnOutputRole: resolvedDeliveryRole, }); const dispatch = resolveFollowUpDispatch({ - source: 'owner-delivery-success', + source: 'delivery-success', nextTurnAction, + completedRole: resolvedDeliveryRole, }); if ( dispatch.kind === 'enqueue' && @@ -376,8 +377,12 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { scheduled, }, scheduled - ? 'Queued paired follow-up after successful owner delivery' - : 'Skipped duplicate paired follow-up after successful owner delivery while task state was unchanged', + ? resolvedDeliveryRole === 'owner' + ? 'Queued paired follow-up after successful owner delivery' + : 'Queued paired follow-up after successful reviewer/arbiter delivery' + : resolvedDeliveryRole === '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', ); } }