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

fix: route arbiter resolutions through reviewer (#195)

Browse Source
This commit is contained in:
Eyejoker
2026-05-29 23:19:42 +09:00
committed by GitHub
parent fb87e94c5a
commit d0ca76f776
9 changed files with 306 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/message-runtime-prompts.ts
View File

@@ -212,6 +212,7 @@ export function buildFinalizePendingPrompt(_args: {
}): string {
return `The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result.
If you intend to close this paired turn now, your first line must be TASK_DONE.
If the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE.
Do not use STEP_DONE only because a broader roadmap still has remaining work; close the approved slice and continue the next slice in a new owner turn.
Use STEP_DONE only when this same approved scope still needs additional owner changes and another review pass.
If your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.`;
}

5
src/message-runtime.test.ts
View File

@@ -1702,7 +1702,8 @@ describe('createMessageRuntime', () => {
expect(input.prompt).toBe(
'The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result.\n' +
'If you intend to close this paired turn now, your first line must be TASK_DONE.\n' +
'If the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE.\n' +
'Do not use STEP_DONE only because a broader roadmap still has remaining work; close the approved slice and continue the next slice in a new owner turn.\n' +
'Use STEP_DONE only when this same approved scope still needs additional owner changes and another review pass.\n' +
'If your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.',
);
expect(input.prompt).not.toContain("Reviewer's final assessment:");
@@ -1811,7 +1812,7 @@ describe('createMessageRuntime', () => {
vi.mocked(agentRunner.runAgentProcess).mockImplementation(
async (_group, input, _onProcess, onOutput) => {
expect(input.prompt).toBe(
'The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result.\nIf you intend to close this paired turn now, your first line must be TASK_DONE.\nIf the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE.\nIf your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.',
'The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result.\nIf you intend to close this paired turn now, your first line must be TASK_DONE.\nDo not use STEP_DONE only because a broader roadmap still has remaining work; close the approved slice and continue the next slice in a new owner turn.\nUse STEP_DONE only when this same approved scope still needs additional owner changes and another review pass.\nIf your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.',
);
expect(input.prompt).not.toContain('리뷰 승인 요약');
expect(input.prompt).not.toContain('DONE\n승인합니다.');

44
src/paired-execution-context-arbiter.ts
View File

@@ -1,9 +1,10 @@
import { ARBITER_DEADLOCK_THRESHOLD } from './config.js';
import { logger } from './logger.js';
import { transitionPairedTaskStatus } from './paired-task-status.js';
import { classifyArbiterVerdict } from './paired-verdict.js';
import type { PairedTask } from './types.js';
const ARBITER_RESOLUTION_ROUND_TRIP_COUNT = 0;
export function handleFailedArbiterExecution(args: {
task: PairedTask;
taskId: string;
@@ -50,6 +51,27 @@ export function handleArbiterCompletion(args: {
switch (arbiterVerdict) {
case 'proceed':
transitionPairedTaskStatus({
taskId,
currentStatus: 'in_arbitration',
nextStatus: 'review_ready',
expectedUpdatedAt: task.updated_at,
updatedAt: now,
patch: {
round_trip_count: ARBITER_RESOLUTION_ROUND_TRIP_COUNT,
owner_failure_count: 0,
owner_step_done_streak: 0,
finalize_step_done_count: 0,
empty_step_done_streak: 0,
arbiter_verdict: arbiterVerdict,
arbiter_requested_at: null,
},
});
logger.info(
{ taskId, arbiterVerdict },
'Arbiter proceeded — returning task to reviewer for approval',
);
return;
case 'revise':
case 'reset':
transitionPairedTaskStatus({
@@ -59,13 +81,18 @@ export function handleArbiterCompletion(args: {
expectedUpdatedAt: task.updated_at,
updatedAt: now,
patch: {
round_trip_count: Math.max(0, ARBITER_DEADLOCK_THRESHOLD - 1),
round_trip_count: ARBITER_RESOLUTION_ROUND_TRIP_COUNT,
owner_failure_count: 0,
owner_step_done_streak: 0,
finalize_step_done_count: 0,
empty_step_done_streak: 0,
arbiter_verdict: arbiterVerdict,
arbiter_requested_at: null,
},
});
logger.info(
{ taskId, arbiterVerdict },
'Arbiter resolved deadlock — resuming ping-pong',
'Arbiter requested owner changes — resuming owner flow',
);
return;
case 'escalate':
@@ -86,17 +113,22 @@ export function handleArbiterCompletion(args: {
transitionPairedTaskStatus({
taskId,
currentStatus: 'in_arbitration',
nextStatus: 'active',
nextStatus: 'review_ready',
expectedUpdatedAt: task.updated_at,
updatedAt: now,
patch: {
round_trip_count: Math.max(0, ARBITER_DEADLOCK_THRESHOLD - 1),
round_trip_count: ARBITER_RESOLUTION_ROUND_TRIP_COUNT,
owner_failure_count: 0,
owner_step_done_streak: 0,
finalize_step_done_count: 0,
empty_step_done_streak: 0,
arbiter_verdict: 'unknown',
arbiter_requested_at: null,
},
});
logger.warn(
{ taskId, summary: summary?.slice(0, 200) },
'Arbiter verdict unrecognized — falling back to proceed',
'Arbiter verdict unrecognized — returning task to reviewer as proceed fallback',
);
return;
}

16
src/paired-execution-context-reviewer.ts
View File

@@ -47,7 +47,15 @@ export function handleFailedReviewerExecution(args: {
updatedAt: now,
patch: {
...(signal.kind === 'request_owner_finalize'
? { source_ref: approvedSourceRef }
? {
source_ref: approvedSourceRef,
owner_failure_count: 0,
owner_step_done_streak: 0,
finalize_step_done_count: 0,
empty_step_done_streak: 0,
arbiter_verdict: null,
arbiter_requested_at: null,
}
: {}),
...(signal.kind === 'complete'
? { completion_reason: signal.completionReason }
@@ -119,6 +127,12 @@ export function handleReviewerCompletion(args: {
updatedAt: now,
patch: {
source_ref: approvedSourceRef,
owner_failure_count: 0,
owner_step_done_streak: 0,
finalize_step_done_count: 0,
empty_step_done_streak: 0,
arbiter_verdict: null,
arbiter_requested_at: null,
},
});
logger.info(

232
src/paired-execution-context-routing.test.ts Normal file
View File

@@ -0,0 +1,232 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock('./db.js', () => {
const updatePairedTask = vi.fn();
return {
getPairedTaskById: vi.fn(),
getPairedWorkspace: vi.fn(),
updatePairedTask,
updatePairedTaskIfUnchanged: vi.fn((id, _expectedUpdatedAt, updates) => {
updatePairedTask(id, updates);
return true;
}),
releasePairedTaskExecutionLease: vi.fn(),
};
});
vi.mock('./paired-workspace-manager.js', () => ({
isOwnerWorkspaceRepairNeededError: vi.fn(() => false),
markPairedTaskReviewReady: vi.fn(),
prepareReviewerWorkspaceForExecution: vi.fn(),
provisionOwnerWorkspaceForPairedTask: vi.fn(),
}));
vi.mock('./logger.js', () => ({
logger: {
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
},
}));
import * as config from './config.js';
import * as db from './db.js';
import { completePairedExecutionContext } from './paired-execution-context.js';
import type { PairedTask } from './types.js';
function buildPairedTask(overrides: Partial<PairedTask> = {}): PairedTask {
return {
id: 'task-1',
chat_jid: 'dc:test',
group_folder: 'ejclaw',
owner_service_id: config.CODEX_MAIN_SERVICE_ID,
reviewer_service_id: config.REVIEWER_SERVICE_ID_FOR_TYPE,
title: null,
source_ref: 'HEAD',
plan_notes: null,
review_requested_at: null,
round_trip_count: 0,
owner_failure_count: 0,
owner_step_done_streak: 0,
finalize_step_done_count: 0,
task_done_then_user_reopen_count: 0,
empty_step_done_streak: 0,
status: 'active',
arbiter_verdict: null,
arbiter_requested_at: null,
completion_reason: null,
created_at: '2026-03-28T00:00:00.000Z',
updated_at: '2026-03-28T00:00:00.000Z',
...overrides,
};
}
describe('paired execution routing loop guards', () => {
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(db.getPairedWorkspace).mockReturnValue(undefined);
});
it('clears stale owner loop state when reviewer approves normally', () => {
vi.mocked(db.getPairedTaskById).mockReturnValue(
buildPairedTask({
status: 'in_review',
source_ref: 'approved-ref',
owner_failure_count: 1,
owner_step_done_streak: 3,
finalize_step_done_count: 1,
empty_step_done_streak: 2,
arbiter_verdict: 'proceed',
arbiter_requested_at: '2026-03-28T00:00:01.000Z',
}),
);
completePairedExecutionContext({
taskId: 'task-1',
role: 'reviewer',
status: 'succeeded',
summary: 'TASK_DONE\n승인',
});
expect(db.updatePairedTask).toHaveBeenCalledWith(
'task-1',
expect.objectContaining({
status: 'merge_ready',
source_ref: 'approved-ref',
owner_failure_count: 0,
owner_step_done_streak: 0,
finalize_step_done_count: 0,
empty_step_done_streak: 0,
arbiter_verdict: null,
arbiter_requested_at: null,
}),
);
});
it('clears stale owner loop state when reviewer approval is recovered from a failed run', () => {
vi.mocked(db.getPairedTaskById).mockReturnValue(
buildPairedTask({
status: 'in_review',
source_ref: 'approved-ref',
owner_step_done_streak: 2,
empty_step_done_streak: 2,
arbiter_verdict: 'proceed',
arbiter_requested_at: '2026-03-28T00:00:01.000Z',
}),
);
completePairedExecutionContext({
taskId: 'task-1',
role: 'reviewer',
status: 'failed',
summary: 'DONE\n승인',
});
expect(db.updatePairedTask).toHaveBeenCalledWith(
'task-1',
expect.objectContaining({
status: 'merge_ready',
owner_step_done_streak: 0,
empty_step_done_streak: 0,
arbiter_verdict: null,
arbiter_requested_at: null,
}),
);
});
it('routes arbiter PROCEED back to reviewer instead of owner ping-pong', () => {
vi.mocked(db.getPairedTaskById).mockReturnValue(
buildPairedTask({
status: 'in_arbitration',
round_trip_count: config.ARBITER_DEADLOCK_THRESHOLD,
owner_step_done_streak: 3,
finalize_step_done_count: 1,
empty_step_done_streak: 4,
arbiter_requested_at: '2026-03-28T00:00:01.000Z',
}),
);
completePairedExecutionContext({
taskId: 'task-1',
role: 'arbiter',
status: 'succeeded',
summary: 'PROCEED\nReviewer should approve.',
});
expect(db.updatePairedTask).toHaveBeenCalledWith(
'task-1',
expect.objectContaining({
status: 'review_ready',
round_trip_count: 0,
owner_failure_count: 0,
owner_step_done_streak: 0,
finalize_step_done_count: 0,
empty_step_done_streak: 0,
arbiter_verdict: 'proceed',
arbiter_requested_at: null,
}),
);
});
it('routes unknown arbiter verdicts to reviewer as a proceed fallback', () => {
vi.mocked(db.getPairedTaskById).mockReturnValue(
buildPairedTask({
status: 'in_arbitration',
round_trip_count: config.ARBITER_DEADLOCK_THRESHOLD,
owner_step_done_streak: 3,
empty_step_done_streak: 4,
arbiter_requested_at: '2026-03-28T00:00:01.000Z',
}),
);
completePairedExecutionContext({
taskId: 'task-1',
role: 'arbiter',
status: 'succeeded',
summary: 'No formal verdict, but this should not re-run owner.',
});
expect(db.updatePairedTask).toHaveBeenCalledWith(
'task-1',
expect.objectContaining({
status: 'review_ready',
round_trip_count: 0,
owner_step_done_streak: 0,
empty_step_done_streak: 0,
arbiter_verdict: 'unknown',
arbiter_requested_at: null,
}),
);
});
it('keeps arbiter REVISE on owner flow while clearing stale loop counters', () => {
vi.mocked(db.getPairedTaskById).mockReturnValue(
buildPairedTask({
status: 'in_arbitration',
round_trip_count: config.ARBITER_DEADLOCK_THRESHOLD,
owner_step_done_streak: 3,
empty_step_done_streak: 4,
arbiter_requested_at: '2026-03-28T00:00:01.000Z',
}),
);
completePairedExecutionContext({
taskId: 'task-1',
role: 'arbiter',
status: 'succeeded',
summary: 'REVISE\nOwner must fix this.',
});
expect(db.updatePairedTask).toHaveBeenCalledWith(
'task-1',
expect.objectContaining({
status: 'active',
owner_step_done_streak: 0,
empty_step_done_streak: 0,
arbiter_verdict: 'revise',
arbiter_requested_at: null,
}),
);
});
});

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

@@ -1296,6 +1296,8 @@ describe('paired execution context', () => {
buildPairedTask({
status: 'in_review',
source_ref: 'stale-ref',
owner_step_done_streak: 2,
empty_step_done_streak: 2,
}),
);
vi.mocked(db.getPairedWorkspace).mockImplementation((_taskId, role) =>
@@ -1314,6 +1316,8 @@ describe('paired execution context', () => {
expect.objectContaining({
status: 'merge_ready',
source_ref: approvedSourceRef,
owner_step_done_streak: 0,
empty_step_done_streak: 0,
}),
);
});

7
src/paired-task-status.ts
View File

@@ -23,7 +23,12 @@ export const ALLOWED_PAIRED_STATUS_TRANSITIONS: Record<
merge_ready: new Set(['active', 'arbiter_requested', 'completed']),
completed: new Set(),
arbiter_requested: new Set(['in_arbitration', 'completed']),
in_arbitration: new Set(['active', 'arbiter_requested', 'completed']),
in_arbitration: new Set([
'active',
'review_ready',
'arbiter_requested',
'completed',
]),
};
export function assertPairedTaskStatusTransition(args: {

1
src/paired-verdict.test.ts
View File

@@ -69,6 +69,7 @@ describe('paired verdict parser', () => {
),
).toBe('reset');
expect(classifyArbiterVerdict('ESCALATE\nblocked')).toBe('escalate');
expect(classifyArbiterVerdict('CONTINUE\nsame as proceed')).toBe('proceed');
expect(classifyArbiterVerdict('no verdict here')).toBe('unknown');
});
});

7
src/paired-verdict.ts
View File

@@ -75,10 +75,13 @@ export function classifyArbiterVerdict(
if (!cleaned) return 'unknown';
const firstLine = cleaned.split('\n')[0].trim();
const verdictMatch = firstLine.match(
/\*{0,2}(?:VERDICT\s*[:—-]\s*)?(PROCEED|REVISE|RESET|ESCALATE)\*{0,2}/i,
/\*{0,2}(?:VERDICT\s*[:—-]\s*)?(PROCEED|REVISE|RESET|ESCALATE|CONTINUE)\*{0,2}/i,
);
if (verdictMatch) {
return verdictMatch[1].toLowerCase() as ArbiterVerdictResult;
const normalized = verdictMatch[1].toLowerCase();
return normalized === 'continue'
? 'proceed'
: (normalized as ArbiterVerdictResult);
}
return 'unknown';
}