From 8ae9e383c07eee690744f0d784f50811af371a64 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Mon, 6 Apr 2026 21:09:46 +0900 Subject: [PATCH] Avoid duplicate paired follow-up scheduling --- src/message-agent-executor-rules.test.ts | 22 +++ src/message-agent-executor-rules.ts | 4 +- src/message-agent-executor.test.ts | 165 +++++++++++++++++++++++ 3 files changed, 188 insertions(+), 3 deletions(-) diff --git a/src/message-agent-executor-rules.test.ts b/src/message-agent-executor-rules.test.ts index e10a6fd..7ad449a 100644 --- a/src/message-agent-executor-rules.test.ts +++ b/src/message-agent-executor-rules.test.ts @@ -129,4 +129,26 @@ describe('message-agent-executor-rules', () => { }), ).toBe('pending'); }); + + it('does not request a generic follow-up after successful owner output moved the task to review_ready', () => { + expect( + resolvePairedFollowUpQueueAction({ + completedRole: 'owner', + executionStatus: 'succeeded', + sawOutput: true, + taskStatus: 'review_ready', + }), + ).toBe('none'); + }); + + it('does not request a generic follow-up after successful reviewer output moved the task back to active', () => { + expect( + resolvePairedFollowUpQueueAction({ + completedRole: 'reviewer', + executionStatus: 'succeeded', + sawOutput: true, + taskStatus: 'active', + }), + ).toBe('none'); + }); }); diff --git a/src/message-agent-executor-rules.ts b/src/message-agent-executor-rules.ts index c03c33a..2671e10 100644 --- a/src/message-agent-executor-rules.ts +++ b/src/message-agent-executor-rules.ts @@ -127,9 +127,7 @@ export function resolvePairedFollowUpQueueAction(args: { return args.completedRole === 'reviewer' && args.taskStatus === 'merge_ready' ? 'skip-inline-finalize' - : args.taskStatus !== 'completed' - ? 'generic' - : 'none'; + : 'none'; } const shouldRequeuePendingPairedTurn = diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index e617f0f..602078d 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -1238,6 +1238,171 @@ describe('runAgentForGroup room memory', () => { expect(deps.queue.enqueueMessageCheck).not.toHaveBeenCalled(); }); + it('does not enqueue a generic follow-up when owner output already moved the task to review_ready', async () => { + const group = { ...makeGroup(), folder: 'test-group' }; + const deps = makeDeps(); + + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-owner-review-ready', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: null, + status: 'active', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-31T00:00:00.000Z', + updated_at: '2026-03-31T00:00:00.000Z', + }, + workspace: null, + envOverrides: {}, + }); + vi.mocked(db.getPairedTaskById).mockReturnValue({ + id: 'paired-task-owner-review-ready', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: null, + status: 'review_ready', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-31T00:00:00.000Z', + updated_at: '2026-03-31T00:00:00.000Z', + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + result: 'DONE_WITH_CONCERNS\nowner complete', + output: { + visibility: 'public', + text: 'DONE_WITH_CONCERNS\nowner complete', + }, + phase: 'final', + }); + return { + status: 'success', + result: 'DONE_WITH_CONCERNS\nowner complete', + }; + }, + ); + + const result = await runAgentForGroup(deps, { + group, + prompt: 'please implement', + chatJid: 'group@test', + runId: 'run-owner-review-ready', + onOutput: async () => {}, + }); + + expect(result).toBe('success'); + expect(deps.queue.enqueueMessageCheck).not.toHaveBeenCalled(); + }); + + it('does not enqueue a generic follow-up when reviewer output already returned the task to active', async () => { + const group = { ...makeGroup(), folder: 'test-group' }; + const deps = makeDeps(); + + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_agent_type: 'codex', + reviewer_agent_type: 'claude-code', + arbiter_agent_type: null, + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-reviewer-active', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 1, + review_requested_at: '2026-03-31T00:00:00.000Z', + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-31T00:00:00.000Z', + updated_at: '2026-03-31T00:00:00.000Z', + }, + workspace: null, + envOverrides: {}, + }); + vi.mocked(db.getPairedTaskById).mockReturnValue({ + id: 'paired-task-reviewer-active', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 1, + review_requested_at: '2026-03-31T00:00:00.000Z', + status: 'active', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-31T00:00:00.000Z', + updated_at: '2026-03-31T00:00:00.000Z', + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + result: 'DONE_WITH_CONCERNS\nreview complete', + output: { + visibility: 'public', + text: 'DONE_WITH_CONCERNS\nreview complete', + }, + phase: 'final', + }); + return { + status: 'success', + result: 'DONE_WITH_CONCERNS\nreview complete', + }; + }, + ); + + const result = await runAgentForGroup(deps, { + group, + prompt: 'please review', + chatJid: 'group@test', + runId: 'run-reviewer-active', + forcedRole: 'reviewer', + onOutput: async () => {}, + }); + + expect(result).toBe('success'); + expect(deps.queue.enqueueMessageCheck).not.toHaveBeenCalled(); + }); + it('re-enqueues the group when a reviewer fails and the task remains review_ready', async () => { const group = { ...makeGroup(), folder: 'test-group' }; const deps = makeDeps();