From a5368296736cf55b441e5a0497b5f177c9b3e302 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Wed, 29 Apr 2026 17:49:10 +0900 Subject: [PATCH] fix: escalate paired step-done loops --- src/paired-completion-signals.test.ts | 7 +++++ src/paired-completion-signals.ts | 3 ++ src/paired-execution-context-owner.ts | 44 +++++++++++++++++++++++++++ src/paired-execution-context.test.ts | 19 +++++------- 4 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/paired-completion-signals.test.ts b/src/paired-completion-signals.test.ts index 4ae68b1..95f9197 100644 --- a/src/paired-completion-signals.test.ts +++ b/src/paired-completion-signals.test.ts @@ -94,6 +94,13 @@ describe('paired completion signals', () => { deadlockThreshold: 3, }), ).toEqual({ kind: 'request_owner_changes' }); + expect( + resolveReviewerCompletionSignal({ + visibleVerdict: 'step_done', + roundTripCount: 3, + deadlockThreshold: 3, + }), + ).toEqual({ kind: 'request_arbiter' }); expect( resolveReviewerCompletionSignal({ visibleVerdict: 'done', diff --git a/src/paired-completion-signals.ts b/src/paired-completion-signals.ts index e8573a1..b1ab590 100644 --- a/src/paired-completion-signals.ts +++ b/src/paired-completion-signals.ts @@ -72,6 +72,9 @@ export function resolveReviewerCompletionSignal(args: { case 'done': return { kind: 'request_owner_finalize' }; case 'step_done': + if (roundTripCount >= deadlockThreshold) { + return { kind: 'request_arbiter' }; + } return { kind: 'request_owner_changes' }; case 'blocked': case 'needs_context': diff --git a/src/paired-execution-context-owner.ts b/src/paired-execution-context-owner.ts index 310bed5..2d28c96 100644 --- a/src/paired-execution-context-owner.ts +++ b/src/paired-execution-context-owner.ts @@ -341,8 +341,16 @@ export function handleOwnerCompletion(args: { } const ownerVerdict = parseVisibleVerdict(summary); + const workspace = getPairedWorkspace(task.id, 'owner'); + const hasNewChanges = workspace?.workspace_dir + ? hasCodeChangesSinceRef(workspace.workspace_dir, task.source_ref) + : null; const nextOwnerStepDoneStreak = ownerVerdict === 'step_done' ? (task.owner_step_done_streak ?? 0) + 1 : 0; + const nextEmptyStepDoneStreak = + ownerVerdict === 'step_done' && hasNewChanges === false + ? (task.empty_step_done_streak ?? 0) + 1 + : 0; const signal = resolveOwnerCompletionSignal({ phase: 'normal', visibleVerdict: ownerVerdict, @@ -370,6 +378,37 @@ export function handleOwnerCompletion(args: { return; } + if ( + ownerVerdict === 'step_done' && + hasNewChanges === false && + nextEmptyStepDoneStreak >= EMPTY_STEP_DONE_THRESHOLD + ) { + requestArbiterOrEscalate({ + taskId, + currentStatus: task.status, + expectedUpdatedAt: task.updated_at, + now, + arbiterLogMessage: + 'Owner repeated STEP_DONE without code changes — requesting arbiter', + escalateLogMessage: + 'Owner repeated STEP_DONE without code changes — escalating to user', + logContext: { + taskId, + ownerVerdict, + hasNewChanges, + ownerStepDoneStreak: nextOwnerStepDoneStreak, + emptyStepDoneStreak: nextEmptyStepDoneStreak, + summary: summary?.slice(0, 100), + }, + patch: { + owner_failure_count: 0, + owner_step_done_streak: nextOwnerStepDoneStreak, + empty_step_done_streak: nextEmptyStepDoneStreak, + }, + }); + return; + } + if (nextOwnerStepDoneStreak !== (task.owner_step_done_streak ?? 0)) { applyPairedTaskPatch({ taskId, @@ -377,6 +416,7 @@ export function handleOwnerCompletion(args: { updatedAt: now, patch: { owner_step_done_streak: nextOwnerStepDoneStreak, + empty_step_done_streak: nextEmptyStepDoneStreak, }, }); } @@ -385,5 +425,9 @@ export function handleOwnerCompletion(args: { taskId, now, logMessage: 'Auto-triggered reviewer after owner completion', + patch: { + owner_step_done_streak: nextOwnerStepDoneStreak, + empty_step_done_streak: nextEmptyStepDoneStreak, + }, }); } diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index ba33ec2..df28dcb 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -866,7 +866,7 @@ describe('paired execution context', () => { 'task-1', expect.objectContaining({ round_trip_count: 2, - owner_step_done_streak: 0, + owner_step_done_streak: 1, empty_step_done_streak: 0, }), ); @@ -875,7 +875,7 @@ describe('paired execution context', () => { ).toHaveBeenCalledWith('task-1'); }); - it('requests review instead of arbiter when active STEP_DONE repeats without code changes', () => { + it('requests arbiter when active STEP_DONE repeats without code changes', () => { vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true); const repoDir = createCanonicalRepoWithCommit('active step done loop'); @@ -911,20 +911,15 @@ describe('paired execution context', () => { expect(db.updatePairedTask).toHaveBeenCalledWith( 'task-1', expect.objectContaining({ - round_trip_count: 1, - owner_step_done_streak: 0, - empty_step_done_streak: 0, + status: 'arbiter_requested', + arbiter_requested_at: expect.any(String), + owner_step_done_streak: 3, + empty_step_done_streak: 3, }), ); expect( pairedWorkspaceManager.markPairedTaskReviewReady, - ).toHaveBeenCalledWith('task-1'); - expect(db.updatePairedTask).not.toHaveBeenCalledWith( - 'task-1', - expect.objectContaining({ - status: 'arbiter_requested', - }), - ); + ).not.toHaveBeenCalled(); }); it('re-triggers review when the owner reports STEP_DONE during finalize', () => {