claude-bot/EJClaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix paired reviewer finalize flow

Browse Source
This commit is contained in:
ejclaw
2026-04-10 18:45:19 +09:00
parent 8f8cfac58f
commit 446223e3c8
9 changed files with 330 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
src/message-agent-executor-paired.ts
View File

@@ -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,13 +265,30 @@ export function createPairedExecutionLifecycle(args: {
return;
}
const missingReviewerOrArbiterVerdict =
(completedRole === 'reviewer' || completedRole === 'arbiter') &&
pairedExecutionStatus === 'succeeded' &&
!pairedTerminalSummary;
const effectiveStatus =
completedRole === 'owner' &&
pairedExecutionStatus === 'succeeded' &&
!pairedSawOutput
? 'failed'
: 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') {
try {
@@ -278,7 +306,10 @@ export function createPairedExecutionLifecycle(args: {
role: completedRole,
status: effectiveStatus,
runId,
summary: pairedExecutionSummary,
summary:
effectiveStatus === 'succeeded'
? (pairedTerminalSummary ?? pairedExecutionSummary)
: (pairedExecutionSummary ?? pairedTerminalSummary),
});
pairedExecutionCompleted = true;
}

93
src/message-agent-executor.test.ts
View File

@@ -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(),

1
src/message-agent-executor.ts
View File

@@ -194,6 +194,7 @@ export async function runAgentForGroup(
runId,
roomRoleContext,
hasHumanMessage: args.hasHumanMessage,
pairedTurnIdentity: args.pairedTurnIdentity,
});
const preparedTurnTaskUpdatedAt = pairedExecutionContext
? (pairedExecutionContext.claimedTaskUpdatedAt ??

62
src/message-turn-controller.test.ts
View File

@@ -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<void> {
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();
});
});

20
src/message-turn-controller.ts
View File

@@ -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
) {
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() &&

28
src/paired-execution-context-shared.test.ts
View File

@@ -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({

21
src/paired-execution-context-shared.ts
View File

@@ -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';

46
src/paired-execution-context.test.ts
View File

@@ -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',

15
src/paired-execution-context.ts
View File

@@ -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();