From b6c8546f2504694d148515280d7a692cc2447557 Mon Sep 17 00:00:00 2001 From: Eyejoker Date: Sun, 5 Apr 2026 08:01:46 +0900 Subject: [PATCH] Dedup reviewer finals only after actual IPC delivery --- src/index.ts | 17 +++++- src/message-runtime.test.ts | 109 +++++++++++++++++++++++++++++++----- src/message-runtime.ts | 13 ----- 3 files changed, 109 insertions(+), 30 deletions(-) diff --git a/src/index.ts b/src/index.ts index 7e1d162..5fddaf8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -77,6 +77,13 @@ import { initTokenRotation, onTokenRotated, } from './token-rotation.js'; + +function isTerminalStatusMessage(text: string): boolean { + const trimmed = text.trimStart(); + return /^(?:\*\*)?(DONE(?:_WITH_CONCERNS)?|BLOCKED|NEEDS_CONTEXT)\b/.test( + trimmed, + ); +} import { onTokenRefreshed, startTokenRefreshLoop, @@ -472,7 +479,7 @@ async function main(): Promise { editFormattedTrackedChannelMessage(channels, jid, messageId, rawText), }); startIpcWatcher({ - sendMessage: (jid, text, senderRole) => { + sendMessage: async (jid, text, senderRole) => { const route = resolveChannelForDeliveryRole(channels, jid, senderRole); if (!route.channel) throw new Error(`No channel for JID: ${jid}`); logger.info( @@ -487,7 +494,13 @@ async function main(): Promise { }, 'IPC relay routed message to channel', ); - return route.channel.sendMessage(jid, text); + await route.channel.sendMessage(jid, text); + if ( + (senderRole === 'reviewer' || senderRole === 'arbiter') && + isTerminalStatusMessage(text) + ) { + queue.noteDirectTerminalDelivery(jid, senderRole); + } }, nudgeScheduler: nudgeSchedulerLoop, registeredGroups: () => registeredGroups, diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index 599de6f..2a7b3e1 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -1100,7 +1100,6 @@ describe('createMessageRuntime', () => { }; const reviewerChannel = makeChannel(chatJid, 'discord-review', false); const noteDirectTerminalDelivery = vi.fn(); - let recordedReviewerTerminal = false; vi.mocked(config.isClaudeService).mockReturnValue(false); vi.mocked(config.isReviewService).mockReturnValue(false); @@ -1151,20 +1150,12 @@ describe('createMessageRuntime', () => { registerProcess: vi.fn(), closeStdin: vi.fn(), notifyIdle: vi.fn(), - noteDirectTerminalDelivery: vi.fn( - (groupJid: string, senderRole?: string | null) => { - noteDirectTerminalDelivery(groupJid, senderRole); - if (groupJid === chatJid && senderRole === 'reviewer') { - recordedReviewerTerminal = true; - } - }, - ), + noteDirectTerminalDelivery, hasDirectTerminalDeliveryForRun: vi.fn( (groupJid: string, runId: string, senderRole?: string | null) => groupJid === chatJid && runId === 'run-review-direct-terminal-skip' && - senderRole === 'reviewer' && - recordedReviewerTerminal, + senderRole === 'reviewer', ), } as any, getRegisteredGroups: () => ({ [chatJid]: group }), @@ -1183,10 +1174,7 @@ describe('createMessageRuntime', () => { }); expect(result).toBe(true); - expect(noteDirectTerminalDelivery).toHaveBeenCalledWith( - chatJid, - 'reviewer', - ); + expect(noteDirectTerminalDelivery).not.toHaveBeenCalled(); expect(db.createProducedWorkItem).not.toHaveBeenCalled(); expect(reviewerChannel.sendMessage).not.toHaveBeenCalled(); expect(logger.info).toHaveBeenCalledWith( @@ -1199,6 +1187,97 @@ describe('createMessageRuntime', () => { ); }); + it('does not suppress reviewer finals based on streamed terminal output alone', async () => { + const chatJid = 'group@test'; + const group = makeGroup('codex'); + const ownerChannel: Channel = { + ...makeChannel(chatJid), + isOwnMessage: vi.fn((msg) => msg.sender === 'owner-bot@test'), + }; + const reviewerChannel = makeChannel(chatJid, 'discord-review', false); + const noteDirectTerminalDelivery = vi.fn(); + + vi.mocked(config.isClaudeService).mockReturnValue(false); + vi.mocked(config.isReviewService).mockReturnValue(false); + vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true); + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ + id: 'task-review-streamed-terminal', + 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: 0, + 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', + }); + vi.mocked(db.getLastHumanMessageContent).mockReturnValue('리뷰해줘'); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + phase: 'final', + result: '**DONE**\n\n리뷰 승인', + newSessionId: 'session-review-streamed-terminal', + }); + return { + status: 'success', + result: '**DONE**\n\n리뷰 승인', + newSessionId: 'session-review-streamed-terminal', + }; + }, + ); + + 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(), + noteDirectTerminalDelivery, + hasDirectTerminalDeliveryForRun: vi.fn(() => false), + } 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-review-streamed-terminal', + reason: 'messages', + }); + + expect(result).toBe(true); + expect(noteDirectTerminalDelivery).not.toHaveBeenCalled(); + expect(db.createProducedWorkItem).toHaveBeenCalledWith( + expect.objectContaining({ + delivery_role: 'reviewer', + result_payload: '**DONE**\n\n리뷰 승인', + }), + ); + expect(reviewerChannel.sendMessage).toHaveBeenCalledWith( + chatJid, + '**DONE**\n\n리뷰 승인', + ); + }); + it('does not suppress owner finals even if the queue reports a direct terminal delivery for the run', async () => { const chatJid = 'group@test'; const group = makeGroup('codex'); diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 5b827e5..95de8d5 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -504,8 +504,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { const isClaudeCodeAgent = (args.forcedAgentType ?? group.agentType ?? 'claude-code') === 'claude-code'; - let directTerminalDeliveryRecorded = false; - const turnController = new MessageTurnController({ chatJid, group, @@ -572,17 +570,6 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { chatJid, runId, async (result) => { - const raw = result ? getAgentOutputText(result) : null; - if ( - !directTerminalDeliveryRecorded && - raw && - isTerminalStatusMessage(raw) && - (args.deliveryRole === 'reviewer' || - args.deliveryRole === 'arbiter') - ) { - deps.queue.noteDirectTerminalDelivery?.(chatJid, args.deliveryRole); - directTerminalDeliveryRecorded = true; - } await turnController.handleOutput(result); }, {