diff --git a/src/message-agent-executor-paired.ts b/src/message-agent-executor-paired.ts index f782d09..aa4a0f1 100644 --- a/src/message-agent-executor-paired.ts +++ b/src/message-agent-executor-paired.ts @@ -14,6 +14,7 @@ import { completePairedExecutionContext, type PreparedPairedExecutionContext, } from './paired-execution-context.js'; +import { hasRecognizedPairedTerminalSummary } from './paired-execution-context-shared.js'; import { resolvePairedFollowUpQueueAction } from './message-agent-executor-rules.js'; import { enqueuePairedFollowUpAfterEvent } from './message-runtime-follow-up.js'; import type { PairedTurnIdentity } from './paired-turn-identity.js'; @@ -60,7 +61,8 @@ export function createPairedExecutionLifecycle(args: { let pairedExecutionStatus: 'succeeded' | 'failed' = 'failed'; let pairedExecutionSummary: string | null = null; - let pairedFinalOutput: string | null = null; + let pairedTerminalSummary: string | null = null; + let pairedTerminalOutput: string | null = null; let pairedExecutionCompleted = false; let pairedExecutionDelegated = false; let pairedSawOutput = false; @@ -131,12 +133,20 @@ export function createPairedExecutionLifecycle(args: { leaseHeartbeatTimer.unref?.(); } + const rememberTerminalOutputIfRecognized = (outputText: string) => { + if (!hasRecognizedPairedTerminalSummary(completedRole, outputText)) { + return; + } + pairedTerminalSummary = outputText.slice(0, 500); + pairedTerminalOutput = outputText; + }; + const persistPairedTurnOutputIfNeeded = () => { if ( !pairedExecutionContext || pairedTurnOutputPersisted || - !pairedFinalOutput || - pairedFinalOutput.length === 0 + !pairedTerminalOutput || + pairedTerminalOutput.length === 0 ) { return; } @@ -146,7 +156,7 @@ export function createPairedExecutionLifecycle(args: { pairedExecutionContext.task.id, turnNumber, completedRole, - pairedFinalOutput, + pairedTerminalOutput, ); pairedTurnOutputPersisted = true; }; @@ -156,8 +166,8 @@ export function createPairedExecutionLifecycle(args: { completedRole !== 'owner' || !pairedExecutionContext || pairedExecutionCompleted || - !pairedFinalOutput || - pairedFinalOutput.length === 0 + !pairedTerminalOutput || + pairedTerminalOutput.length === 0 ) { return; } @@ -171,7 +181,7 @@ export function createPairedExecutionLifecycle(args: { role: completedRole, status: 'succeeded', runId, - summary: pairedExecutionSummary, + summary: pairedTerminalSummary ?? pairedExecutionSummary, }); pairedExecutionCompleted = true; }; @@ -180,6 +190,7 @@ export function createPairedExecutionLifecycle(args: { updateSummary({ outputText, errorText }) { if (outputText && outputText.length > 0) { pairedExecutionSummary = outputText.slice(0, 500); + rememberTerminalOutputIfRecognized(outputText); return; } @@ -189,7 +200,7 @@ export function createPairedExecutionLifecycle(args: { }, recordFinalOutputBeforeDelivery(outputText) { - pairedFinalOutput = outputText; + rememberTerminalOutputIfRecognized(outputText); completeSuccessfulOwnerTurnBeforeDeliveryIfNeeded(); persistPairedTurnOutputIfNeeded(); }, @@ -254,12 +265,29 @@ export function createPairedExecutionLifecycle(args: { return; } + const missingReviewerOrArbiterVerdict = + (completedRole === 'reviewer' || completedRole === 'arbiter') && + pairedExecutionStatus === 'succeeded' && + !pairedTerminalSummary; const effectiveStatus = completedRole === 'owner' && pairedExecutionStatus === 'succeeded' && !pairedSawOutput ? 'failed' - : pairedExecutionStatus; + : missingReviewerOrArbiterVerdict + ? 'failed' + : pairedExecutionStatus; + + if (missingReviewerOrArbiterVerdict) { + log.warn( + { + pairedTaskId: pairedExecutionContext?.task.id ?? null, + role: completedRole, + summary: pairedExecutionSummary, + }, + 'Downgraded paired reviewer/arbiter completion to failed because no explicit terminal verdict was emitted', + ); + } if (pairedExecutionContext && !pairedExecutionCompleted) { if (effectiveStatus === 'succeeded') { @@ -278,7 +306,10 @@ export function createPairedExecutionLifecycle(args: { role: completedRole, status: effectiveStatus, runId, - summary: pairedExecutionSummary, + summary: + effectiveStatus === 'succeeded' + ? (pairedTerminalSummary ?? pairedExecutionSummary) + : (pairedExecutionSummary ?? pairedTerminalSummary), }); pairedExecutionCompleted = true; } diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 0bd37bb..7e3041f 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -1236,6 +1236,99 @@ describe('runAgentForGroup room memory', () => { ]); }); + it('fails a paired reviewer turn when the run ends after progress-only output without an explicit reviewer verdict', async () => { + const group = { + ...makeGroup(), + folder: 'test-group', + workDir: '/repo/canonical', + }; + + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-reviewer-progress-only', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: '2026-04-10T00:00:00.000Z', + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-04-10T00:00:00.000Z', + updated_at: '2026-04-10T00:00:00.000Z', + }, + workspace: null, + envOverrides: {}, + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + phase: 'progress', + result: + '요청 범위가 실제로 지워졌는지와 검증 근거가 맞는지 확인하겠습니다.', + output: { + visibility: 'public', + text: '요청 범위가 실제로 지워졌는지와 검증 근거가 맞는지 확인하겠습니다.', + }, + } as AgentOutput); + await onOutput?.({ + status: 'success', + phase: 'progress', + result: + '우선 변경 파일과 현재 계약 노출 지점을 직접 읽고, 필요하면 전용 검증도 다시 돌리겠습니다.', + output: { + visibility: 'public', + text: '우선 변경 파일과 현재 계약 노출 지점을 직접 읽고, 필요하면 전용 검증도 다시 돌리겠습니다.', + }, + } as AgentOutput); + return { + status: 'success', + result: null, + } as AgentOutput; + }, + ); + + await expect( + runAgentForGroup(makeDeps(), { + group, + prompt: 'review please', + chatJid: 'group@test', + runId: 'run-reviewer-progress-only', + forcedRole: 'reviewer', + forcedAgentType: 'codex', + pairedTurnIdentity: { + turnId: + 'paired-task-reviewer-progress-only:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-reviewer-progress-only', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + }), + ).resolves.toBe('success'); + + expect( + pairedExecutionContext.completePairedExecutionContext, + ).toHaveBeenCalledWith( + expect.objectContaining({ + taskId: 'paired-task-reviewer-progress-only', + role: 'reviewer', + status: 'failed', + summary: expect.stringContaining( + '우선 변경 파일과 현재 계약 노출 지점을', + ), + }), + ); + }); + it('logs streamed activity with resolved execution attribution', async () => { const group = { ...makeGroup(), diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 970d004..7626324 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -194,6 +194,7 @@ export async function runAgentForGroup( runId, roomRoleContext, hasHumanMessage: args.hasHumanMessage, + pairedTurnIdentity: args.pairedTurnIdentity, }); const preparedTurnTaskUpdatedAt = pairedExecutionContext ? (pairedExecutionContext.claimedTaskUpdatedAt ?? diff --git a/src/message-turn-controller.test.ts b/src/message-turn-controller.test.ts index 2eafd85..0b33f2d 100644 --- a/src/message-turn-controller.test.ts +++ b/src/message-turn-controller.test.ts @@ -59,6 +59,28 @@ function makeTurnIdentity(): PairedTurnIdentity { }; } +function makeTurnIdentityWithRole( + role: PairedTurnIdentity['role'], +): PairedTurnIdentity { + return { + ...makeTurnIdentity(), + role, + intentKind: + role === 'arbiter' + ? 'arbiter-turn' + : role === 'owner' + ? 'owner-turn' + : 'reviewer-turn', + turnId: `task-1:2026-04-10T14:22:00.000Z:${ + role === 'arbiter' + ? 'arbiter-turn' + : role === 'owner' + ? 'owner-turn' + : 'reviewer-turn' + }`, + }; +} + async function flushAsync(): Promise { await new Promise((resolve) => setTimeout(resolve, 0)); } @@ -221,4 +243,44 @@ describe('MessageTurnController outbound audit logging', () => { ]), ); }); + + it('does not promote paired reviewer progress-only output to a final message when no explicit verdict was emitted', async () => { + const channel = makeChannel(); + const deliverFinalText = vi.fn().mockResolvedValue(true); + + const controller = new MessageTurnController({ + chatJid: 'dc:test-room', + group: makeGroup(), + runId: 'run-reviewer-progress-only', + channel, + idleTimeout: 1_000, + failureFinalText: '실패', + isClaudeCodeAgent: false, + clearSession: vi.fn(), + requestClose: vi.fn(), + deliverFinalText, + deliveryRole: 'reviewer', + deliveryServiceId: 'codex-review', + pairedTurnIdentity: makeTurnIdentityWithRole('reviewer'), + }); + + await controller.start(); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: + '요청 범위가 실제로 지워졌는지와 검증 근거가 맞는지 확인하겠습니다.', + } as any); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: + '우선 변경 파일과 현재 계약 노출 지점을 직접 읽고, 필요하면 전용 검증도 다시 돌리겠습니다.', + } as any); + await flushAsync(); + await controller.finish('success'); + + expect(channel.sendAndTrack).toHaveBeenCalledTimes(1); + expect(deliverFinalText).not.toHaveBeenCalled(); + }); }); diff --git a/src/message-turn-controller.ts b/src/message-turn-controller.ts index 7a5665c..7b09542 100644 --- a/src/message-turn-controller.ts +++ b/src/message-turn-controller.ts @@ -5,6 +5,7 @@ import { formatOutbound } from './router.js'; import { shouldResetSessionOnAgentFailure } from './session-recovery.js'; import { TASK_STATUS_MESSAGE_PREFIX } from './task-watch-status.js'; import { formatElapsedKorean } from './utils.js'; +import { hasRecognizedPairedTerminalSummary } from './paired-execution-context-shared.js'; import type { PairedTurnIdentity } from './paired-turn-identity.js'; import { normalizeAgentOutputPhase, @@ -338,11 +339,30 @@ export class MessageTurnController { !this.hadError && this.latestProgressTextForFinal ) { - this.log.info( - 'Sending a separate final message from the last progress output after agent completion', - ); - await this.finalizeProgressMessage(); - await this.deliverFinalText(this.latestProgressTextForFinal); + const pairedRole = this.options.pairedTurnIdentity?.role ?? null; + const shouldPromoteToFinal = + !pairedRole || + hasRecognizedPairedTerminalSummary( + pairedRole, + this.latestProgressTextForFinal, + ); + if (shouldPromoteToFinal) { + this.log.info( + 'Sending a separate final message from the last progress output after agent completion', + ); + await this.finalizeProgressMessage(); + await this.deliverFinalText(this.latestProgressTextForFinal); + } else { + this.log.info( + { + pairedRole, + turnId: this.options.pairedTurnIdentity?.turnId ?? null, + }, + 'Skipped separate final message promotion because paired progress did not contain an explicit terminal verdict', + ); + await this.finalizeProgressMessage(); + this.latestProgressTextForFinal = null; + } } else if ( this.visiblePhase === 'progress' && !this.terminalObserved() && diff --git a/src/paired-execution-context-shared.test.ts b/src/paired-execution-context-shared.test.ts index b19568b..9c791c6 100644 --- a/src/paired-execution-context-shared.test.ts +++ b/src/paired-execution-context-shared.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { + hasRecognizedPairedTerminalSummary, parseVisibleVerdict, resolveOwnerCompletionSignal, resolveReviewerCompletionSignal, @@ -18,6 +19,33 @@ describe('paired execution context shared verdict helpers', () => { expect(parseVisibleVerdict('random prose')).toBe('continue'); }); + it('recognizes paired terminal summaries only when the role-specific verdict is explicit', () => { + expect( + hasRecognizedPairedTerminalSummary('owner', 'plain owner update'), + ).toBe(true); + expect( + hasRecognizedPairedTerminalSummary( + 'reviewer', + 'DONE_WITH_CONCERNS\nfollow-up detail', + ), + ).toBe(true); + expect( + hasRecognizedPairedTerminalSummary( + 'reviewer', + '요청 범위를 실제로 지워졌는지 확인하겠습니다.', + ), + ).toBe(false); + expect( + hasRecognizedPairedTerminalSummary('arbiter', 'PROCEED\n승인합니다.'), + ).toBe(true); + expect( + hasRecognizedPairedTerminalSummary( + 'arbiter', + '대화와 현재 코드 상태를 대조해 먼저 확인하겠습니다.', + ), + ).toBe(false); + }); + it('maps normal owner completion verdicts to reviewer or arbiter signals', () => { expect( resolveOwnerCompletionSignal({ diff --git a/src/paired-execution-context-shared.ts b/src/paired-execution-context-shared.ts index 58d2f95..9066afd 100644 --- a/src/paired-execution-context-shared.ts +++ b/src/paired-execution-context-shared.ts @@ -3,7 +3,7 @@ import { execFileSync } from 'child_process'; import { isArbiterEnabled } from './config.js'; import { updatePairedTaskIfUnchanged } from './db.js'; import { logger } from './logger.js'; -import type { PairedTaskStatus } from './types.js'; +import type { PairedRoomRole, PairedTaskStatus } from './types.js'; export type VisibleVerdict = | 'done' @@ -149,6 +149,25 @@ export function classifyArbiterVerdict( return 'unknown'; } +export function hasRecognizedPairedTerminalSummary( + role: PairedRoomRole, + summary: string | null | undefined, +): boolean { + if (!summary || summary.trim().length === 0) { + return false; + } + + if (role === 'owner') { + return true; + } + + if (role === 'reviewer') { + return parseVisibleVerdict(summary) !== 'continue'; + } + + return classifyArbiterVerdict(summary) !== 'unknown'; +} + export function resolveCanonicalSourceRef(workDir: string): string { const treeHash = resolveCanonicalTreeHash(workDir); return treeHash || 'HEAD'; diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index d6071dd..c3409c0 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -46,6 +46,7 @@ import { preparePairedExecutionContext, } from './paired-execution-context.js'; import * as pairedWorkspaceManager from './paired-workspace-manager.js'; +import type { PairedTurnIdentity } from './paired-turn-identity.js'; import type { PairedTask, PairedWorkspace, @@ -335,6 +336,51 @@ describe('paired execution context', () => { }); }); + it('reuses the persisted reviewer turn revision when a claimed handoff re-enters an already in_review task', () => { + const pairedTurnIdentity: PairedTurnIdentity = { + turnId: 'task-1:2026-03-28T00:00:00.000Z:reviewer-turn', + taskId: 'task-1', + taskUpdatedAt: '2026-03-28T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }; + + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue( + buildPairedTask({ + status: 'in_review', + review_requested_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:00:05.000Z', + }), + ); + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'in_review', + review_requested_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:00:05.000Z', + }), + ); + vi.mocked( + pairedWorkspaceManager.prepareReviewerWorkspaceForExecution, + ).mockReturnValue({ + workspace: buildWorkspace('reviewer', '/tmp/paired/task-1/reviewer'), + autoRefreshed: false, + }); + + const result = preparePairedExecutionContext({ + group, + chatJid: 'dc:test', + runId: 'run-reviewer-handoff-reentry', + roomRoleContext: reviewerContext, + pairedTurnIdentity, + }); + + expect(result?.claimedTaskUpdatedAt).toBe('2026-03-28T00:00:00.000Z'); + expect(db.updatePairedTask).not.toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ status: 'in_review' }), + ); + }); + it('does not change task state when the reviewer snapshot is not ready', () => { vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ id: 'task-1', diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index cdef09e..9b6f6f8 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -46,6 +46,7 @@ import { prepareReviewerWorkspaceForExecution, provisionOwnerWorkspaceForPairedTask, } from './paired-workspace-manager.js'; +import type { PairedTurnIdentity } from './paired-turn-identity.js'; import type { AgentType, PairedRoomRole, @@ -193,6 +194,7 @@ export function preparePairedExecutionContext(args: { runId: string; roomRoleContext?: RoomRoleContext; hasHumanMessage?: boolean; + pairedTurnIdentity?: PairedTurnIdentity; }): PreparedPairedExecutionContext | undefined { const { group, chatJid, roomRoleContext } = args; if (!roomRoleContext || !group.workDir) { @@ -210,7 +212,18 @@ export function preparePairedExecutionContext(args: { } const latestTask = getPairedTaskById(task.id) ?? task; - const claimedTaskUpdatedAt = latestTask.updated_at; + const claimedTaskUpdatedAt = + args.pairedTurnIdentity && + args.pairedTurnIdentity.taskId === latestTask.id && + args.pairedTurnIdentity.role === roomRoleContext.role && + ((roomRoleContext.role === 'reviewer' && + (latestTask.status === 'review_ready' || + latestTask.status === 'in_review')) || + (roomRoleContext.role === 'arbiter' && + (latestTask.status === 'arbiter_requested' || + latestTask.status === 'in_arbitration'))) + ? args.pairedTurnIdentity.taskUpdatedAt + : latestTask.updated_at; let workspace: PairedWorkspace | null = null; let blockMessage: string | undefined; const now = new Date().toISOString();