Avoid duplicate merge_ready finalize follow-ups
This commit is contained in:
@@ -795,7 +795,7 @@ describe('createMessageRuntime', () => {
|
|||||||
expect(enqueueMessageCheck).not.toHaveBeenCalled();
|
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 chatJid = 'group@test';
|
||||||
const group = makeGroup('codex');
|
const group = makeGroup('codex');
|
||||||
const ownerChannel = makeChannel(chatJid);
|
const ownerChannel = makeChannel(chatJid);
|
||||||
@@ -874,10 +874,19 @@ describe('createMessageRuntime', () => {
|
|||||||
'reviewer final retry',
|
'reviewer final retry',
|
||||||
);
|
);
|
||||||
expect(ownerChannel.sendMessage).not.toHaveBeenCalled();
|
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 chatJid = 'group@test';
|
||||||
const group = makeGroup('claude-code');
|
const group = makeGroup('claude-code');
|
||||||
const ownerChannel = makeChannel(chatJid);
|
const ownerChannel = makeChannel(chatJid);
|
||||||
@@ -958,7 +967,16 @@ describe('createMessageRuntime', () => {
|
|||||||
'fallback reviewer final retry',
|
'fallback reviewer final retry',
|
||||||
);
|
);
|
||||||
expect(ownerChannel.sendMessage).not.toHaveBeenCalled();
|
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 () => {
|
it('does not inject filtered raw bot finals into workspace-based review prompts', async () => {
|
||||||
|
|||||||
@@ -897,10 +897,30 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
|||||||
openWorkItem,
|
openWorkItem,
|
||||||
);
|
);
|
||||||
if (!delivered) return false;
|
if (!delivered) return false;
|
||||||
|
const skipQueuedFollowUpForInlineFinalize =
|
||||||
|
hasReviewerLease(chatJid) &&
|
||||||
|
deliveryRole === 'reviewer' &&
|
||||||
|
pendingTask?.status === 'merge_ready';
|
||||||
// Keep stale final delivery isolated from the next conversational turn.
|
// Keep stale final delivery isolated from the next conversational turn.
|
||||||
// A follow-up run will pick up any queued user messages or paired-room
|
// A follow-up run will pick up any queued user messages or paired-room
|
||||||
// auto-review/finalize transitions after this retry succeeds.
|
// 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;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user