fix: escalate finalize blockers without reviewer bounce
This commit is contained in:
@@ -32,6 +32,7 @@ vi.mock('./logger.js', () => ({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
import * as db from './db.js';
|
import * as db from './db.js';
|
||||||
|
import * as config from './config.js';
|
||||||
import {
|
import {
|
||||||
completePairedExecutionContext,
|
completePairedExecutionContext,
|
||||||
preparePairedExecutionContext,
|
preparePairedExecutionContext,
|
||||||
@@ -443,6 +444,38 @@ describe('paired execution context', () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it.each(['BLOCKED', 'NEEDS_CONTEXT'])(
|
||||||
|
'escalates immediately when owner reports %s during finalize without arbiter',
|
||||||
|
(summary) => {
|
||||||
|
vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(false);
|
||||||
|
|
||||||
|
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||||
|
buildPairedTask({
|
||||||
|
status: 'merge_ready',
|
||||||
|
round_trip_count: 1,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
completePairedExecutionContext({
|
||||||
|
taskId: 'task-1',
|
||||||
|
role: 'owner',
|
||||||
|
status: 'succeeded',
|
||||||
|
summary,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||||
|
'task-1',
|
||||||
|
expect.objectContaining({
|
||||||
|
status: 'completed',
|
||||||
|
completion_reason: 'escalated',
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
pairedWorkspaceManager.markPairedTaskReviewReady,
|
||||||
|
).not.toHaveBeenCalled();
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
it('records source_ref when reviewer verdict DONE arrives via failed fallback', () => {
|
it('records source_ref when reviewer verdict DONE arrives via failed fallback', () => {
|
||||||
const repoDir = createCanonicalRepoWithCommit('reviewed');
|
const repoDir = createCanonicalRepoWithCommit('reviewed');
|
||||||
const approvedSourceRef = resolveTreeRef(repoDir);
|
const approvedSourceRef = resolveTreeRef(repoDir);
|
||||||
|
|||||||
@@ -422,10 +422,42 @@ export function completePairedExecutionContext(args: {
|
|||||||
// detached HEAD, discovered issue). Respect the owner's verdict.
|
// detached HEAD, discovered issue). Respect the owner's verdict.
|
||||||
const ownerVerdict = classifyReviewerVerdict(args.summary);
|
const ownerVerdict = classifyReviewerVerdict(args.summary);
|
||||||
if (
|
if (
|
||||||
ownerVerdict === 'done_with_concerns' ||
|
|
||||||
ownerVerdict === 'blocked' ||
|
ownerVerdict === 'blocked' ||
|
||||||
ownerVerdict === 'needs_context'
|
ownerVerdict === 'needs_context'
|
||||||
) {
|
) {
|
||||||
|
if (isArbiterEnabled()) {
|
||||||
|
updatePairedTask(taskId, {
|
||||||
|
status: 'arbiter_requested',
|
||||||
|
arbiter_requested_at: now,
|
||||||
|
updated_at: now,
|
||||||
|
});
|
||||||
|
logger.info(
|
||||||
|
{
|
||||||
|
taskId,
|
||||||
|
ownerVerdict,
|
||||||
|
summary: args.summary?.slice(0, 100),
|
||||||
|
},
|
||||||
|
'Owner blocked during finalize — requesting arbiter',
|
||||||
|
);
|
||||||
|
} else {
|
||||||
|
updatePairedTask(taskId, {
|
||||||
|
status: 'completed',
|
||||||
|
completion_reason: 'escalated',
|
||||||
|
updated_at: now,
|
||||||
|
});
|
||||||
|
logger.info(
|
||||||
|
{
|
||||||
|
taskId,
|
||||||
|
ownerVerdict,
|
||||||
|
summary: args.summary?.slice(0, 100),
|
||||||
|
},
|
||||||
|
'Owner blocked during finalize — escalating to user',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (ownerVerdict === 'done_with_concerns') {
|
||||||
// Check deadlock threshold before looping back — prevents
|
// Check deadlock threshold before looping back — prevents
|
||||||
// merge_ready ↔ active infinite oscillation.
|
// merge_ready ↔ active infinite oscillation.
|
||||||
if (task.round_trip_count >= ARBITER_DEADLOCK_THRESHOLD) {
|
if (task.round_trip_count >= ARBITER_DEADLOCK_THRESHOLD) {
|
||||||
|
|||||||
Reference in New Issue
Block a user