diff --git a/src/message-runtime-flow.test.ts b/src/message-runtime-flow.test.ts index 280f51b..bea70a6 100644 --- a/src/message-runtime-flow.test.ts +++ b/src/message-runtime-flow.test.ts @@ -102,7 +102,8 @@ describe('executeBotOnlyPairedFollowUpAction', () => { expect(first).toBe(true); expect(second).toBe(true); - expect(closeStdin).toHaveBeenCalledTimes(2); + expect(closeStdin).toHaveBeenCalledTimes(1); + expect(closeStdin).toHaveBeenCalledWith(); expect(enqueue).toHaveBeenCalledTimes(1); expect(log.info).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/src/message-runtime-flow.ts b/src/message-runtime-flow.ts index ca311e2..4833822 100644 --- a/src/message-runtime-flow.ts +++ b/src/message-runtime-flow.ts @@ -400,8 +400,10 @@ export async function executeBotOnlyPairedFollowUpAction(args: { return true; } - closeStdin(); const scheduled = schedulePairedFollowUp(action.task, action.intentKind); + if (scheduled) { + closeStdin(); + } log.info( { chatJid, diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index 109bf99..b381bef 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -219,7 +219,10 @@ import { } from './message-runtime-flow.js'; import * as config from './config.js'; import { logger } from './logger.js'; -import { resetPairedFollowUpScheduleState } from './paired-follow-up-scheduler.js'; +import { + resetPairedFollowUpScheduleState, + schedulePairedFollowUpOnce, +} from './paired-follow-up-scheduler.js'; import * as serviceRouting from './service-routing.js'; import type { Channel, RegisteredGroup } from './types.js'; @@ -2205,6 +2208,120 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead ); }); + it('re-enqueues owner after arbiter delivery even when a stale owner follow-up key exists for the prior task revision', 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 staleOwnerFollowUpEnqueue = vi.fn(); + const pairedTask = { + id: 'task-arbiter-delivery-owner-follow-up-stale-key', + 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:10.000Z', + } as any; + + schedulePairedFollowUpOnce({ + chatJid, + runId: 'run-stale-owner-follow-up', + task: { + id: pairedTask.id, + status: 'active', + round_trip_count: 1, + updated_at: '2026-03-30T00:00:05.000Z', + }, + intentKind: 'owner-follow-up', + enqueue: staleOwnerFollowUpEnqueue, + }); + + expect(staleOwnerFollowUpEnqueue).toHaveBeenCalledTimes(1); + + 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'; + pairedTask.arbiter_verdict = 'revise'; + pairedTask.updated_at = '2026-03-30T00:00:20.000Z'; + await onOutput?.({ + status: 'success', + phase: 'final', + result: 'DONE_WITH_CONCERNS\narbiter says revise', + newSessionId: 'session-arbiter-delivery-owner-follow-up-stale-key', + }); + return { + status: 'success', + result: 'DONE_WITH_CONCERNS\narbiter says revise', + newSessionId: 'session-arbiter-delivery-owner-follow-up-stale-key', + }; + }, + ); + + 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-stale-key', + 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-stale-key', + completedRole: 'arbiter', + taskId: 'task-arbiter-delivery-owner-follow-up-stale-key', + taskStatus: 'active', + scheduled: true, + }), + '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/paired-follow-up-scheduler.test.ts b/src/paired-follow-up-scheduler.test.ts index 0469b4c..e20f6df 100644 --- a/src/paired-follow-up-scheduler.test.ts +++ b/src/paired-follow-up-scheduler.test.ts @@ -19,6 +19,7 @@ describe('paired follow-up scheduler', () => { id: 'task-1', status: 'review_ready', round_trip_count: 1, + updated_at: '2026-03-30T00:00:00.000Z', } as const; const first = schedulePairedFollowUpOnce({ @@ -47,6 +48,7 @@ describe('paired follow-up scheduler', () => { id: 'task-1', status: 'review_ready', round_trip_count: 1, + updated_at: '2026-03-30T00:00:00.000Z', } as const; const first = schedulePairedFollowUpOnce({ @@ -78,6 +80,7 @@ describe('paired follow-up scheduler', () => { id: 'task-1', status: 'review_ready', round_trip_count: 1, + updated_at: '2026-03-30T00:00:00.000Z', } as const, intentKind: 'reviewer-turn', enqueue, @@ -89,6 +92,7 @@ describe('paired follow-up scheduler', () => { id: 'task-1', status: 'review_ready', round_trip_count: 2, + updated_at: '2026-03-30T00:00:01.000Z', } as const, intentKind: 'reviewer-turn', enqueue, @@ -105,9 +109,43 @@ describe('paired follow-up scheduler', () => { taskId: 'task-1', taskStatus: 'review_ready', roundTripCount: 3, + taskUpdatedAt: '2026-03-30T00:00:00.000Z', intentKind: 'reviewer-turn', }), - ).toBe('task-1:review_ready:3:reviewer-turn'); + ).toBe('task-1:review_ready:3:2026-03-30T00:00:00.000Z:reviewer-turn'); + }); + + it('keeps different task revisions schedulable even when status and round trip are unchanged', () => { + const enqueue = vi.fn(); + + const first = schedulePairedFollowUpOnce({ + chatJid: 'group@test', + runId: 'run-1', + task: { + id: 'task-1', + status: 'active', + round_trip_count: 1, + updated_at: '2026-03-30T00:00:00.000Z', + } as const, + intentKind: 'owner-follow-up', + enqueue, + }); + const second = schedulePairedFollowUpOnce({ + chatJid: 'group@test', + runId: 'run-2', + task: { + id: 'task-1', + status: 'active', + round_trip_count: 1, + updated_at: '2026-03-30T00:00:10.000Z', + } as const, + intentKind: 'owner-follow-up', + enqueue, + }); + + expect(first).toBe(true); + expect(second).toBe(true); + expect(enqueue).toHaveBeenCalledTimes(2); }); it('allows the same follow-up again after the TTL expires', () => { @@ -119,6 +157,7 @@ describe('paired follow-up scheduler', () => { id: 'task-1', status: 'review_ready', round_trip_count: 1, + updated_at: '2026-03-30T00:00:00.000Z', } as const; const first = schedulePairedFollowUpOnce({ diff --git a/src/paired-follow-up-scheduler.ts b/src/paired-follow-up-scheduler.ts index dfae7fa..9a91f4b 100644 --- a/src/paired-follow-up-scheduler.ts +++ b/src/paired-follow-up-scheduler.ts @@ -8,7 +8,7 @@ export type ScheduledPairedFollowUpIntentKind = type ScheduledPairedFollowUpTask = Pick< PairedTask, - 'id' | 'status' | 'round_trip_count' + 'id' | 'status' | 'round_trip_count' | 'updated_at' >; export const SCHEDULED_PAIRED_FOLLOW_UP_TTL_MS = 10 * 60 * 1000; @@ -26,12 +26,14 @@ export function buildPairedFollowUpKey(args: { taskId: string; taskStatus: PairedTaskStatus | null; roundTripCount: number; + taskUpdatedAt: string | null | undefined; intentKind: ScheduledPairedFollowUpIntentKind; }): string { return [ args.taskId, args.taskStatus ?? 'unknown', String(args.roundTripCount), + args.taskUpdatedAt ?? 'unknown', args.intentKind, ].join(':'); } @@ -52,6 +54,7 @@ export function schedulePairedFollowUpOnce(args: { taskId: args.task.id, taskStatus: args.task.status, roundTripCount: args.task.round_trip_count, + taskUpdatedAt: args.task.updated_at, intentKind: args.intentKind, }), ].join(':');