Merge pull request #106 from phj1081/codex/owner/ejclaw
fix: escalate paired step-done loops
This commit is contained in:
@@ -94,6 +94,13 @@ describe('paired completion signals', () => {
|
|||||||
deadlockThreshold: 3,
|
deadlockThreshold: 3,
|
||||||
}),
|
}),
|
||||||
).toEqual({ kind: 'request_owner_changes' });
|
).toEqual({ kind: 'request_owner_changes' });
|
||||||
|
expect(
|
||||||
|
resolveReviewerCompletionSignal({
|
||||||
|
visibleVerdict: 'step_done',
|
||||||
|
roundTripCount: 3,
|
||||||
|
deadlockThreshold: 3,
|
||||||
|
}),
|
||||||
|
).toEqual({ kind: 'request_arbiter' });
|
||||||
expect(
|
expect(
|
||||||
resolveReviewerCompletionSignal({
|
resolveReviewerCompletionSignal({
|
||||||
visibleVerdict: 'done',
|
visibleVerdict: 'done',
|
||||||
|
|||||||
@@ -72,6 +72,9 @@ export function resolveReviewerCompletionSignal(args: {
|
|||||||
case 'done':
|
case 'done':
|
||||||
return { kind: 'request_owner_finalize' };
|
return { kind: 'request_owner_finalize' };
|
||||||
case 'step_done':
|
case 'step_done':
|
||||||
|
if (roundTripCount >= deadlockThreshold) {
|
||||||
|
return { kind: 'request_arbiter' };
|
||||||
|
}
|
||||||
return { kind: 'request_owner_changes' };
|
return { kind: 'request_owner_changes' };
|
||||||
case 'blocked':
|
case 'blocked':
|
||||||
case 'needs_context':
|
case 'needs_context':
|
||||||
|
|||||||
@@ -341,8 +341,16 @@ export function handleOwnerCompletion(args: {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const ownerVerdict = parseVisibleVerdict(summary);
|
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 =
|
const nextOwnerStepDoneStreak =
|
||||||
ownerVerdict === 'step_done' ? (task.owner_step_done_streak ?? 0) + 1 : 0;
|
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({
|
const signal = resolveOwnerCompletionSignal({
|
||||||
phase: 'normal',
|
phase: 'normal',
|
||||||
visibleVerdict: ownerVerdict,
|
visibleVerdict: ownerVerdict,
|
||||||
@@ -370,6 +378,37 @@ export function handleOwnerCompletion(args: {
|
|||||||
return;
|
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)) {
|
if (nextOwnerStepDoneStreak !== (task.owner_step_done_streak ?? 0)) {
|
||||||
applyPairedTaskPatch({
|
applyPairedTaskPatch({
|
||||||
taskId,
|
taskId,
|
||||||
@@ -377,6 +416,7 @@ export function handleOwnerCompletion(args: {
|
|||||||
updatedAt: now,
|
updatedAt: now,
|
||||||
patch: {
|
patch: {
|
||||||
owner_step_done_streak: nextOwnerStepDoneStreak,
|
owner_step_done_streak: nextOwnerStepDoneStreak,
|
||||||
|
empty_step_done_streak: nextEmptyStepDoneStreak,
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -385,5 +425,9 @@ export function handleOwnerCompletion(args: {
|
|||||||
taskId,
|
taskId,
|
||||||
now,
|
now,
|
||||||
logMessage: 'Auto-triggered reviewer after owner completion',
|
logMessage: 'Auto-triggered reviewer after owner completion',
|
||||||
|
patch: {
|
||||||
|
owner_step_done_streak: nextOwnerStepDoneStreak,
|
||||||
|
empty_step_done_streak: nextEmptyStepDoneStreak,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -866,7 +866,7 @@ describe('paired execution context', () => {
|
|||||||
'task-1',
|
'task-1',
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
round_trip_count: 2,
|
round_trip_count: 2,
|
||||||
owner_step_done_streak: 0,
|
owner_step_done_streak: 1,
|
||||||
empty_step_done_streak: 0,
|
empty_step_done_streak: 0,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
@@ -875,7 +875,7 @@ describe('paired execution context', () => {
|
|||||||
).toHaveBeenCalledWith('task-1');
|
).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);
|
vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true);
|
||||||
|
|
||||||
const repoDir = createCanonicalRepoWithCommit('active step done loop');
|
const repoDir = createCanonicalRepoWithCommit('active step done loop');
|
||||||
@@ -911,20 +911,15 @@ describe('paired execution context', () => {
|
|||||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||||
'task-1',
|
'task-1',
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
round_trip_count: 1,
|
status: 'arbiter_requested',
|
||||||
owner_step_done_streak: 0,
|
arbiter_requested_at: expect.any(String),
|
||||||
empty_step_done_streak: 0,
|
owner_step_done_streak: 3,
|
||||||
|
empty_step_done_streak: 3,
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
expect(
|
expect(
|
||||||
pairedWorkspaceManager.markPairedTaskReviewReady,
|
pairedWorkspaceManager.markPairedTaskReviewReady,
|
||||||
).toHaveBeenCalledWith('task-1');
|
).not.toHaveBeenCalled();
|
||||||
expect(db.updatePairedTask).not.toHaveBeenCalledWith(
|
|
||||||
'task-1',
|
|
||||||
expect.objectContaining({
|
|
||||||
status: 'arbiter_requested',
|
|
||||||
}),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it('re-triggers review when the owner reports STEP_DONE during finalize', () => {
|
it('re-triggers review when the owner reports STEP_DONE during finalize', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user