From f5e9400b9b7c4cbbc9438be1564d943477739f20 Mon Sep 17 00:00:00 2001 From: Eyejoker Date: Sun, 5 Apr 2026 01:08:02 +0900 Subject: [PATCH] Avoid duplicate merge_ready finalize follow-ups --- src/message-runtime.test.ts | 26 ++++++++++++++++++++++---- src/message-runtime.ts | 22 +++++++++++++++++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index 53b7474..928cfd7 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -795,7 +795,7 @@ describe('createMessageRuntime', () => { expect(enqueueMessageCheck).not.toHaveBeenCalled(); }); - it('retries a stale reviewer work item via the persisted delivery role even after task status changed', async () => { + it('does not queue a second merge_ready follow-up after delivering a stale reviewer work item', async () => { const chatJid = 'group@test'; const group = makeGroup('codex'); const ownerChannel = makeChannel(chatJid); @@ -874,10 +874,19 @@ describe('createMessageRuntime', () => { 'reviewer final retry', ); expect(ownerChannel.sendMessage).not.toHaveBeenCalled(); - expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); + expect(enqueueMessageCheck).not.toHaveBeenCalled(); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + workItemId: 101, + chatJid, + deliveryRole: 'reviewer', + pendingTaskStatus: 'merge_ready', + }), + 'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff', + ); }); - it('retries a stale fallback reviewer work item even when the room owner agent type is different', async () => { + it('does not queue a second merge_ready follow-up for fallback reviewer deliveries either', async () => { const chatJid = 'group@test'; const group = makeGroup('claude-code'); const ownerChannel = makeChannel(chatJid); @@ -958,7 +967,16 @@ describe('createMessageRuntime', () => { 'fallback reviewer final retry', ); expect(ownerChannel.sendMessage).not.toHaveBeenCalled(); - expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); + expect(enqueueMessageCheck).not.toHaveBeenCalled(); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + workItemId: 102, + chatJid, + deliveryRole: 'reviewer', + pendingTaskStatus: 'merge_ready', + }), + 'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff', + ); }); it('does not inject filtered raw bot finals into workspace-based review prompts', async () => { diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 3a4c323..947235a 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -897,10 +897,30 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { openWorkItem, ); if (!delivered) return false; + const skipQueuedFollowUpForInlineFinalize = + hasReviewerLease(chatJid) && + deliveryRole === 'reviewer' && + pendingTask?.status === 'merge_ready'; // Keep stale final delivery isolated from the next conversational turn. // A follow-up run will pick up any queued user messages or paired-room // auto-review/finalize transitions after this retry succeeds. - deps.queue.enqueueMessageCheck(chatJid); + // + // merge_ready is special: the delivered reviewer bot message itself + // triggers the inline finalize path. Queueing a generic follow-up here + // can race with that path and produce a second owner finalize turn. + if (skipQueuedFollowUpForInlineFinalize) { + log.info( + { + workItemId: openWorkItem.id, + chatJid, + deliveryRole, + pendingTaskStatus: pendingTask?.status ?? null, + }, + 'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff', + ); + } else { + deps.queue.enqueueMessageCheck(chatJid); + } return true; }