diff --git a/src/db/base-schema.ts b/src/db/base-schema.ts index 5db1b72..b3d2447 100644 --- a/src/db/base-schema.ts +++ b/src/db/base-schema.ts @@ -142,6 +142,7 @@ export function applyBaseSchema(database: Database): void { review_requested_at TEXT, round_trip_count INTEGER NOT NULL DEFAULT 0, owner_failure_count INTEGER NOT NULL DEFAULT 0, + reviewer_failure_count INTEGER NOT NULL DEFAULT 0, owner_step_done_streak INTEGER NOT NULL DEFAULT 0, finalize_step_done_count INTEGER NOT NULL DEFAULT 0, task_done_then_user_reopen_count INTEGER NOT NULL DEFAULT 0, diff --git a/src/db/bootstrap.test.ts b/src/db/bootstrap.test.ts index 47d8451..ad288de 100644 --- a/src/db/bootstrap.test.ts +++ b/src/db/bootstrap.test.ts @@ -43,6 +43,7 @@ function getExpectedSchemaMigrations(): Array<{ { version: 16, name: 'room_skill_overrides' }, { version: 17, name: 'scheduled_task_room_role' }, { version: 18, name: 'paired_turn_output_attachments' }, + { version: 19, name: 'reviewer_failure_count' }, ]; } diff --git a/src/db/migrations/019_reviewer-failure-count.ts b/src/db/migrations/019_reviewer-failure-count.ts new file mode 100644 index 0000000..5d9f20e --- /dev/null +++ b/src/db/migrations/019_reviewer-failure-count.ts @@ -0,0 +1,23 @@ +import type { Database } from 'bun:sqlite'; + +import { tableHasColumn } from './helpers.js'; +import type { SchemaMigrationDefinition } from './types.js'; + +export const REVIEWER_FAILURE_COUNT_MIGRATION: SchemaMigrationDefinition = { + version: 19, + name: 'reviewer_failure_count', + apply(database: Database) { + if (!tableHasColumn(database, 'paired_tasks', 'reviewer_failure_count')) { + database.exec(` + ALTER TABLE paired_tasks + ADD COLUMN reviewer_failure_count INTEGER NOT NULL DEFAULT 0 + `); + } + + database.exec(` + UPDATE paired_tasks + SET reviewer_failure_count = 0 + WHERE reviewer_failure_count IS NULL + `); + }, +}; diff --git a/src/db/migrations/index.ts b/src/db/migrations/index.ts index 38f8702..ebf67e5 100644 --- a/src/db/migrations/index.ts +++ b/src/db/migrations/index.ts @@ -18,6 +18,7 @@ import { TURN_PROGRESS_TEXT_MIGRATION } from './015_turn-progress-text.js'; import { ROOM_SKILL_OVERRIDES_MIGRATION } from './016_room-skill-overrides.js'; import { SCHEDULED_TASK_ROOM_ROLE_MIGRATION } from './017_scheduled-task-room-role.js'; import { PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION } from './018_paired-turn-output-attachments.js'; +import { REVIEWER_FAILURE_COUNT_MIGRATION } from './019_reviewer-failure-count.js'; import type { SchemaMigrationArgs, SchemaMigrationDefinition, @@ -44,6 +45,7 @@ const ORDERED_SCHEMA_MIGRATIONS: readonly SchemaMigrationDefinition[] = [ ROOM_SKILL_OVERRIDES_MIGRATION, SCHEDULED_TASK_ROOM_ROLE_MIGRATION, PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION, + REVIEWER_FAILURE_COUNT_MIGRATION, ]; function ensureSchemaMigrationsTable(database: Database): void { diff --git a/src/db/paired-state.ts b/src/db/paired-state.ts index 6c5ba78..b5dea17 100644 --- a/src/db/paired-state.ts +++ b/src/db/paired-state.ts @@ -50,6 +50,7 @@ export type PairedTaskUpdates = Partial< | 'review_requested_at' | 'round_trip_count' | 'owner_failure_count' + | 'reviewer_failure_count' | 'owner_step_done_streak' | 'finalize_step_done_count' | 'task_done_then_user_reopen_count' @@ -176,6 +177,7 @@ export function createPairedTaskInDatabase( review_requested_at, round_trip_count, owner_failure_count, + reviewer_failure_count, owner_step_done_streak, finalize_step_done_count, task_done_then_user_reopen_count, @@ -187,7 +189,7 @@ export function createPairedTaskInDatabase( created_at, updated_at ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `, ) .run( @@ -205,6 +207,7 @@ export function createPairedTaskInDatabase( task.review_requested_at, task.round_trip_count, task.owner_failure_count ?? 0, + task.reviewer_failure_count ?? 0, task.owner_step_done_streak ?? 0, task.finalize_step_done_count ?? 0, task.task_done_then_user_reopen_count ?? 0, @@ -339,6 +342,10 @@ export function updatePairedTaskInDatabase( fields.push('owner_failure_count = ?'); values.push(updates.owner_failure_count); } + if (updates.reviewer_failure_count !== undefined) { + fields.push('reviewer_failure_count = ?'); + values.push(updates.reviewer_failure_count); + } if (updates.owner_step_done_streak !== undefined) { fields.push('owner_step_done_streak = ?'); values.push(updates.owner_step_done_streak); @@ -418,6 +425,10 @@ export function updatePairedTaskIfUnchangedInDatabase( fields.push('owner_failure_count = ?'); values.push(updates.owner_failure_count); } + if (updates.reviewer_failure_count !== undefined) { + fields.push('reviewer_failure_count = ?'); + values.push(updates.reviewer_failure_count); + } if (updates.owner_step_done_streak !== undefined) { fields.push('owner_step_done_streak = ?'); values.push(updates.owner_step_done_streak); diff --git a/src/paired-arbiter-request.ts b/src/paired-arbiter-request.ts index b653a75..8abdb4d 100644 --- a/src/paired-arbiter-request.ts +++ b/src/paired-arbiter-request.ts @@ -18,6 +18,7 @@ export function requestArbiterOrEscalate(args: { review_requested_at?: string | null; round_trip_count?: number; owner_failure_count?: number; + reviewer_failure_count?: number; owner_step_done_streak?: number; finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; diff --git a/src/paired-execution-context-owner.ts b/src/paired-execution-context-owner.ts index 2d28c96..e9f0bd8 100644 --- a/src/paired-execution-context-owner.ts +++ b/src/paired-execution-context-owner.ts @@ -293,6 +293,7 @@ function maybeAutoTriggerReviewerAfterOwnerCompletion(args: { patch: { round_trip_count: task.round_trip_count + 1, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, empty_step_done_streak: 0, ...args.patch, diff --git a/src/paired-execution-context-reviewer.ts b/src/paired-execution-context-reviewer.ts index 8dc0d58..9927e4f 100644 --- a/src/paired-execution-context-reviewer.ts +++ b/src/paired-execution-context-reviewer.ts @@ -2,7 +2,10 @@ import { ARBITER_DEADLOCK_THRESHOLD } from './config.js'; import { getPairedWorkspace } from './db.js'; import { logger } from './logger.js'; import { requestArbiterOrEscalate } from './paired-arbiter-request.js'; -import { transitionPairedTaskStatus } from './paired-task-status.js'; +import { + applyPairedTaskPatch, + transitionPairedTaskStatus, +} from './paired-task-status.js'; import { resolveReviewerCompletionSignal, resolveReviewerFailureSignal, @@ -11,6 +14,17 @@ import { resolveCanonicalSourceRef } from './paired-source-ref.js'; import { parseVisibleVerdict } from './paired-verdict.js'; import type { PairedTask } from './types.js'; +/** + * Maximum consecutive reviewer executions that may produce no verdict + * before we stop the ping-pong loop. Mirrors the owner-side failure cap and + * closes the "reviewer silently fails forever" hole: codex sometimes emits a + * final result with no visible text, the runtime marks it failed, the + * next-turn scheduler re-queues another reviewer-turn, and the original + * round_trip_count never increments — so the existing round-trip / arbiter + * caps never fire. + */ +const REVIEWER_FAILURE_ESCALATION_THRESHOLD = 2; + export function handleFailedReviewerExecution(args: { task: PairedTask; taskId: string; @@ -75,6 +89,35 @@ export function handleFailedReviewerExecution(args: { } } + // Reviewer produced no usable verdict and no recognised infrastructure + // failure. Track consecutive flakes — after the threshold, stop the + // ping-pong loop instead of preserving review_ready forever. + const nextFailureCount = (task.reviewer_failure_count ?? 0) + 1; + + if (nextFailureCount >= REVIEWER_FAILURE_ESCALATION_THRESHOLD) { + requestArbiterOrEscalate({ + taskId, + currentStatus: task.status, + expectedUpdatedAt: task.updated_at, + now, + arbiterLogMessage: + 'Reviewer failed repeatedly without a visible verdict — requesting arbiter', + escalateLogMessage: + 'Reviewer failed repeatedly without a visible verdict — escalating to user', + logContext: { + taskId, + role: 'reviewer', + previousStatus: task.status, + reviewerFailureCount: nextFailureCount, + summary: summary?.slice(0, 160), + }, + patch: { + reviewer_failure_count: nextFailureCount, + }, + }); + return; + } + const fallbackStatus = task.status === 'in_review' || task.status === 'review_ready' ? 'review_ready' @@ -86,17 +129,30 @@ export function handleFailedReviewerExecution(args: { nextStatus: fallbackStatus, expectedUpdatedAt: task.updated_at, updatedAt: now, - }); - logger.warn( - { - taskId, - role: 'reviewer', - previousStatus: task.status, - nextStatus: fallbackStatus, + patch: { + reviewer_failure_count: nextFailureCount, }, - 'Preserved reviewer task in review-ready state after failed execution', - ); + }); + } else { + applyPairedTaskPatch({ + taskId, + expectedUpdatedAt: task.updated_at, + updatedAt: now, + patch: { + reviewer_failure_count: nextFailureCount, + }, + }); } + logger.warn( + { + taskId, + role: 'reviewer', + previousStatus: task.status, + reviewerFailureCount: nextFailureCount, + summary: summary?.slice(0, 160), + }, + 'Preserved reviewer task in review-ready state after failed execution', + ); } export function handleReviewerCompletion(args: { @@ -128,6 +184,7 @@ export function handleReviewerCompletion(args: { patch: { source_ref: approvedSourceRef, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, finalize_step_done_count: 0, empty_step_done_streak: 0, @@ -164,6 +221,7 @@ export function handleReviewerCompletion(args: { arbiterLogMessage, escalateLogMessage, logContext: { taskId, verdict, summary: summary?.slice(0, 100) }, + patch: { reviewer_failure_count: 0 }, }); return; } @@ -175,6 +233,7 @@ export function handleReviewerCompletion(args: { nextStatus: 'active', expectedUpdatedAt: task.updated_at, updatedAt: now, + patch: { reviewer_failure_count: 0 }, }); logger.info( { taskId, verdict }, diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index 216f56e..3dca9f7 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -141,6 +141,7 @@ function createActiveTaskForRoom(args: { review_requested_at: null, round_trip_count: 0, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, finalize_step_done_count: 0, task_done_then_user_reopen_count: 0, @@ -474,6 +475,7 @@ export function preparePairedExecutionContext(args: { ? { round_trip_count: 0, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, empty_step_done_streak: 0, } @@ -490,6 +492,7 @@ export function preparePairedExecutionContext(args: { ? { round_trip_count: 0, owner_failure_count: 0, + reviewer_failure_count: 0, owner_step_done_streak: 0, empty_step_done_streak: 0, } diff --git a/src/paired-task-status.ts b/src/paired-task-status.ts index 1325552..fd521d1 100644 --- a/src/paired-task-status.ts +++ b/src/paired-task-status.ts @@ -62,6 +62,7 @@ export function transitionPairedTaskStatus(args: { review_requested_at?: string | null; round_trip_count?: number; owner_failure_count?: number; + reviewer_failure_count?: number; owner_step_done_streak?: number; finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; @@ -110,6 +111,7 @@ export function applyPairedTaskPatch(args: { review_requested_at?: string | null; round_trip_count?: number; owner_failure_count?: number; + reviewer_failure_count?: number; owner_step_done_streak?: number; finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; diff --git a/src/types.ts b/src/types.ts index 7c78c22..fdc1089 100644 --- a/src/types.ts +++ b/src/types.ts @@ -128,6 +128,7 @@ export interface PairedTask { review_requested_at: string | null; round_trip_count: number; owner_failure_count?: number | null; + reviewer_failure_count?: number | null; owner_step_done_streak?: number | null; finalize_step_done_count?: number | null; task_done_then_user_reopen_count?: number | null;