diff --git a/src/config.ts b/src/config.ts index 8a113e0..f176824 100644 --- a/src/config.ts +++ b/src/config.ts @@ -95,6 +95,14 @@ export const AGENT_LANGUAGE = CONFIG.paired.agentLanguage; export const ARBITER_DEADLOCK_THRESHOLD = CONFIG.paired.arbiterDeadlockThreshold; +/** + * Maximum number of times the arbiter may intervene on a single task before the + * task is escalated to the user instead of re-invoking the arbiter. This bounds + * the owner↔reviewer↔arbiter loop: once the arbiter has ruled this many times + * and the loop still deadlocks, a human is asked to step in. + */ +export const ARBITER_MAX_INTERVENTIONS = CONFIG.paired.arbiterMaxInterventions; + export function isArbiterEnabled(): boolean { return ARBITER_AGENT_TYPE !== undefined; } diff --git a/src/config/load-config.ts b/src/config/load-config.ts index 449542f..ac9ab41 100644 --- a/src/config/load-config.ts +++ b/src/config/load-config.ts @@ -244,6 +244,7 @@ export function loadConfig(): AppConfig { ), agentLanguage: readText('AGENT_LANGUAGE') ?? '', arbiterDeadlockThreshold: readInteger('ARBITER_DEADLOCK_THRESHOLD', 2), + arbiterMaxInterventions: readInteger('ARBITER_MAX_INTERVENTIONS', 1), maxRoundTrips, }, models: { diff --git a/src/config/schema.ts b/src/config/schema.ts index 5f143c3..967592e 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -57,6 +57,7 @@ export interface AppConfig { forceFreshClaudeReviewerSessionInUnsafeHostMode: boolean; agentLanguage: string; arbiterDeadlockThreshold: number; + arbiterMaxInterventions: number; maxRoundTrips: number; }; models: { diff --git a/src/db/base-schema.ts b/src/db/base-schema.ts index b3d2447..2bbd45d 100644 --- a/src/db/base-schema.ts +++ b/src/db/base-schema.ts @@ -147,6 +147,7 @@ export function applyBaseSchema(database: Database): void { finalize_step_done_count INTEGER NOT NULL DEFAULT 0, task_done_then_user_reopen_count INTEGER NOT NULL DEFAULT 0, empty_step_done_streak INTEGER NOT NULL DEFAULT 0, + arbiter_intervention_count INTEGER NOT NULL DEFAULT 0, status TEXT NOT NULL DEFAULT 'active', arbiter_verdict TEXT, arbiter_requested_at TEXT, diff --git a/src/db/migrations/020_arbiter-intervention-count.ts b/src/db/migrations/020_arbiter-intervention-count.ts new file mode 100644 index 0000000..7e3615a --- /dev/null +++ b/src/db/migrations/020_arbiter-intervention-count.ts @@ -0,0 +1,25 @@ +import type { Database } from 'bun:sqlite'; + +import { tableHasColumn } from './helpers.js'; +import type { SchemaMigrationDefinition } from './types.js'; + +export const ARBITER_INTERVENTION_COUNT_MIGRATION: SchemaMigrationDefinition = { + version: 20, + name: 'arbiter_intervention_count', + apply(database: Database) { + if ( + !tableHasColumn(database, 'paired_tasks', 'arbiter_intervention_count') + ) { + database.exec(` + ALTER TABLE paired_tasks + ADD COLUMN arbiter_intervention_count INTEGER NOT NULL DEFAULT 0 + `); + } + + database.exec(` + UPDATE paired_tasks + SET arbiter_intervention_count = 0 + WHERE arbiter_intervention_count IS NULL + `); + }, +}; diff --git a/src/db/migrations/index.ts b/src/db/migrations/index.ts index 4c67c24..533fcec 100644 --- a/src/db/migrations/index.ts +++ b/src/db/migrations/index.ts @@ -19,6 +19,7 @@ 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 { TURN_PROGRESS_TEXT_RECOVERY_MIGRATION } from './019_turn-progress-text-recovery.js'; +import { ARBITER_INTERVENTION_COUNT_MIGRATION } from './020_arbiter-intervention-count.js'; import type { SchemaMigrationArgs, SchemaMigrationDefinition, @@ -46,6 +47,7 @@ const ORDERED_SCHEMA_MIGRATIONS: readonly SchemaMigrationDefinition[] = [ SCHEDULED_TASK_ROOM_ROLE_MIGRATION, PAIRED_TURN_OUTPUT_ATTACHMENTS_MIGRATION, TURN_PROGRESS_TEXT_RECOVERY_MIGRATION, + ARBITER_INTERVENTION_COUNT_MIGRATION, ]; function ensureSchemaMigrationsTable(database: Database): void { diff --git a/src/db/paired-state.ts b/src/db/paired-state.ts index 6c5ba78..d873ede 100644 --- a/src/db/paired-state.ts +++ b/src/db/paired-state.ts @@ -54,6 +54,7 @@ export type PairedTaskUpdates = Partial< | 'finalize_step_done_count' | 'task_done_then_user_reopen_count' | 'empty_step_done_streak' + | 'arbiter_intervention_count' | 'status' | 'arbiter_verdict' | 'arbiter_requested_at' @@ -180,6 +181,7 @@ export function createPairedTaskInDatabase( finalize_step_done_count, task_done_then_user_reopen_count, empty_step_done_streak, + arbiter_intervention_count, status, arbiter_verdict, arbiter_requested_at, @@ -187,7 +189,7 @@ export function createPairedTaskInDatabase( created_at, updated_at ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `, ) .run( @@ -209,6 +211,7 @@ export function createPairedTaskInDatabase( task.finalize_step_done_count ?? 0, task.task_done_then_user_reopen_count ?? 0, task.empty_step_done_streak ?? 0, + task.arbiter_intervention_count ?? 0, task.status, task.arbiter_verdict, task.arbiter_requested_at, @@ -355,6 +358,10 @@ export function updatePairedTaskInDatabase( fields.push('empty_step_done_streak = ?'); values.push(updates.empty_step_done_streak); } + if (updates.arbiter_intervention_count !== undefined) { + fields.push('arbiter_intervention_count = ?'); + values.push(updates.arbiter_intervention_count); + } if (updates.status !== undefined) { fields.push('status = ?'); values.push(updates.status); @@ -434,6 +441,10 @@ export function updatePairedTaskIfUnchangedInDatabase( fields.push('empty_step_done_streak = ?'); values.push(updates.empty_step_done_streak); } + if (updates.arbiter_intervention_count !== undefined) { + fields.push('arbiter_intervention_count = ?'); + values.push(updates.arbiter_intervention_count); + } if (updates.status !== undefined) { fields.push('status = ?'); values.push(updates.status); diff --git a/src/paired-arbiter-request.ts b/src/paired-arbiter-request.ts index b653a75..83dfab0 100644 --- a/src/paired-arbiter-request.ts +++ b/src/paired-arbiter-request.ts @@ -1,4 +1,4 @@ -import { isArbiterEnabled } from './config.js'; +import { ARBITER_MAX_INTERVENTIONS, isArbiterEnabled } from './config.js'; import { logger } from './logger.js'; import { transitionPairedTaskStatus } from './paired-task-status.js'; import type { PairedTaskStatus } from './types.js'; @@ -11,6 +11,13 @@ export function requestArbiterOrEscalate(args: { arbiterLogMessage: string; escalateLogMessage: string; logContext?: Record; + /** + * How many times the arbiter has already intervened on this task. When this + * reaches {@link maxArbiterInterventions} the task is escalated to the user + * instead of re-invoking the arbiter, bounding the owner↔reviewer↔arbiter loop. + */ + arbiterInterventionCount?: number; + maxArbiterInterventions?: number; patch?: { title?: string | null; source_ref?: string | null; @@ -22,6 +29,7 @@ export function requestArbiterOrEscalate(args: { finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; empty_step_done_streak?: number; + arbiter_intervention_count?: number; arbiter_verdict?: string | null; arbiter_requested_at?: string | null; completion_reason?: string | null; @@ -36,7 +44,10 @@ export function requestArbiterOrEscalate(args: { escalateLogMessage, logContext, } = args; - if (isArbiterEnabled()) { + const interventionCount = args.arbiterInterventionCount ?? 0; + const maxInterventions = + args.maxArbiterInterventions ?? ARBITER_MAX_INTERVENTIONS; + if (isArbiterEnabled() && interventionCount < maxInterventions) { const transitioned = transitionPairedTaskStatus({ taskId, currentStatus, @@ -54,6 +65,33 @@ export function requestArbiterOrEscalate(args: { return transitioned; } + if (isArbiterEnabled() && interventionCount >= maxInterventions) { + const transitioned = transitionPairedTaskStatus({ + taskId, + currentStatus, + nextStatus: 'completed', + expectedUpdatedAt, + updatedAt: now, + patch: { + ...args.patch, + arbiter_verdict: 'escalate', + arbiter_requested_at: null, + completion_reason: 'escalated', + }, + }); + if (transitioned) { + logger.info( + { + ...(logContext ?? { taskId }), + arbiterInterventionCount: interventionCount, + maxArbiterInterventions: maxInterventions, + }, + 'Arbiter intervention budget exhausted — escalating to user instead of re-invoking arbiter', + ); + } + return transitioned; + } + const transitioned = transitionPairedTaskStatus({ taskId, currentStatus, diff --git a/src/paired-execution-context-arbiter.ts b/src/paired-execution-context-arbiter.ts index 7308063..3bb276c 100644 --- a/src/paired-execution-context-arbiter.ts +++ b/src/paired-execution-context-arbiter.ts @@ -78,9 +78,19 @@ export function handleArbiterCompletion(args: { const { task, taskId, summary } = args; const now = new Date().toISOString(); const arbiterVerdict = classifyArbiterVerdict(summary); + // Persist a running count of arbiter interventions so the loop can be bounded. + // This must NOT be reset by the round_trip_count reset below — otherwise the + // owner↔reviewer loop could re-trigger the arbiter without limit. + const nextArbiterInterventionCount = + (task.arbiter_intervention_count ?? 0) + 1; logger.info( - { taskId, arbiterVerdict, summary: summary?.slice(0, 200) }, + { + taskId, + arbiterVerdict, + arbiterInterventionCount: nextArbiterInterventionCount, + summary: summary?.slice(0, 200), + }, 'Arbiter verdict rendered', ); @@ -98,6 +108,7 @@ export function handleArbiterCompletion(args: { owner_step_done_streak: 0, finalize_step_done_count: 0, empty_step_done_streak: 0, + arbiter_intervention_count: nextArbiterInterventionCount, arbiter_verdict: arbiterVerdict, arbiter_requested_at: null, }, @@ -121,12 +132,17 @@ export function handleArbiterCompletion(args: { owner_step_done_streak: 0, finalize_step_done_count: 0, empty_step_done_streak: 0, + arbiter_intervention_count: nextArbiterInterventionCount, arbiter_verdict: arbiterVerdict, arbiter_requested_at: null, }, }); logger.info( - { taskId, arbiterVerdict }, + { + taskId, + arbiterVerdict, + arbiterInterventionCount: nextArbiterInterventionCount, + }, 'Arbiter requested owner changes — resuming owner flow', ); return; diff --git a/src/paired-execution-context-owner.ts b/src/paired-execution-context-owner.ts index d551897..3dcf641 100644 --- a/src/paired-execution-context-owner.ts +++ b/src/paired-execution-context-owner.ts @@ -67,6 +67,7 @@ export function handleFailedOwnerExecution(args: { currentStatus: task.status, expectedUpdatedAt: task.updated_at, now, + arbiterInterventionCount: task.arbiter_intervention_count ?? 0, arbiterLogMessage: 'Owner Codex unavailable repeatedly — requesting arbiter', escalateLogMessage: @@ -140,6 +141,7 @@ export function handleFailedOwnerExecution(args: { currentStatus: task.status, expectedUpdatedAt: task.updated_at, now, + arbiterInterventionCount: task.arbiter_intervention_count ?? 0, arbiterLogMessage: 'Owner failed repeatedly without a visible verdict — requesting arbiter', escalateLogMessage: @@ -237,6 +239,7 @@ function handleOwnerFinalizeCompletion(args: { currentStatus: task.status, expectedUpdatedAt: task.updated_at, now, + arbiterInterventionCount: task.arbiter_intervention_count ?? 0, arbiterLogMessage, escalateLogMessage, logContext: { @@ -266,6 +269,7 @@ function handleOwnerFinalizeCompletion(args: { currentStatus: task.status, expectedUpdatedAt: task.updated_at, now, + arbiterInterventionCount: task.arbiter_intervention_count ?? 0, arbiterLogMessage: 'Owner repeated STEP_DONE during finalize without code changes — requesting arbiter', escalateLogMessage: @@ -464,6 +468,7 @@ export function handleOwnerCompletion(args: { currentStatus: task.status, expectedUpdatedAt: task.updated_at, now, + arbiterInterventionCount: task.arbiter_intervention_count ?? 0, arbiterLogMessage: 'Owner blocked/needs_context — requesting arbiter', escalateLogMessage: 'Owner blocked/needs_context — escalating to user', logContext: { @@ -490,6 +495,7 @@ export function handleOwnerCompletion(args: { currentStatus: task.status, expectedUpdatedAt: task.updated_at, now, + arbiterInterventionCount: task.arbiter_intervention_count ?? 0, arbiterLogMessage: 'Owner repeated STEP_DONE without code changes — requesting arbiter', escalateLogMessage: diff --git a/src/paired-execution-context-reviewer.ts b/src/paired-execution-context-reviewer.ts index 4320b58..aaca85c 100644 --- a/src/paired-execution-context-reviewer.ts +++ b/src/paired-execution-context-reviewer.ts @@ -193,6 +193,7 @@ export function handleReviewerCompletion(args: { arbiterLogMessage, escalateLogMessage, logContext: { taskId, verdict, summary: summary?.slice(0, 100) }, + arbiterInterventionCount: task.arbiter_intervention_count ?? 0, }); return; } diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index 696105c..45e5843 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -147,6 +147,7 @@ function buildPairedTask(overrides: Partial = {}): PairedTask { finalize_step_done_count: 0, task_done_then_user_reopen_count: 0, empty_step_done_streak: 0, + arbiter_intervention_count: 0, status: 'active', arbiter_verdict: null, arbiter_requested_at: null, @@ -1224,6 +1225,81 @@ describe('paired execution context', () => { ).not.toHaveBeenCalled(); }); + it('escalates to the user instead of re-invoking the arbiter once the intervention budget is exhausted', () => { + vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true); + + const repoDir = createCanonicalRepoWithCommit('reviewed'); + const approvedSourceRef = resolveTreeRef(repoDir); + fs.writeFileSync(path.join(repoDir, 'README.md'), 'changed again\n'); + execFileSync('git', ['add', 'README.md'], { + cwd: repoDir, + stdio: 'ignore', + }); + execFileSync('git', ['commit', '-m', 'code change'], { + cwd: repoDir, + stdio: 'ignore', + }); + + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'merge_ready', + source_ref: approvedSourceRef, + round_trip_count: config.ARBITER_DEADLOCK_THRESHOLD, + // Arbiter has already intervened the maximum allowed number of times. + arbiter_intervention_count: config.ARBITER_MAX_INTERVENTIONS, + }), + ); + vi.mocked(db.getPairedWorkspace).mockImplementation((_taskId, role) => + role === 'owner' ? buildWorkspace('owner', repoDir) : undefined, + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'owner', + status: 'succeeded', + summary: 'DONE', + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + status: 'completed', + completion_reason: 'escalated', + }), + ); + expect(db.updatePairedTask).not.toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ status: 'arbiter_requested' }), + ); + }); + + it('increments the arbiter intervention count and keeps it across the round-trip reset on REVISE', () => { + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'in_arbitration', + round_trip_count: config.ARBITER_DEADLOCK_THRESHOLD, + arbiter_intervention_count: 0, + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'arbiter', + status: 'succeeded', + summary: 'VERDICT: REVISE\nOwner should address the reviewer feedback.', + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + status: 'active', + round_trip_count: 0, + arbiter_intervention_count: 1, + arbiter_verdict: 'revise', + }), + ); + }); + it.each(['BLOCKED', 'NEEDS_CONTEXT'])( 'escalates immediately when owner reports %s during finalize without arbiter', (summary) => { diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index 2f316fb..f509aa6 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -145,6 +145,7 @@ function createActiveTaskForRoom(args: { finalize_step_done_count: 0, task_done_then_user_reopen_count: 0, empty_step_done_streak: 0, + arbiter_intervention_count: 0, status: 'active', arbiter_verdict: null, arbiter_requested_at: null, @@ -494,6 +495,7 @@ export function preparePairedExecutionContext(args: { owner_failure_count: 0, owner_step_done_streak: 0, empty_step_done_streak: 0, + arbiter_intervention_count: 0, } : {}), }, @@ -510,6 +512,7 @@ export function preparePairedExecutionContext(args: { owner_failure_count: 0, owner_step_done_streak: 0, empty_step_done_streak: 0, + arbiter_intervention_count: 0, } : {}), }, diff --git a/src/paired-task-status.ts b/src/paired-task-status.ts index 1325552..848c799 100644 --- a/src/paired-task-status.ts +++ b/src/paired-task-status.ts @@ -66,6 +66,7 @@ export function transitionPairedTaskStatus(args: { finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; empty_step_done_streak?: number; + arbiter_intervention_count?: number; arbiter_verdict?: string | null; arbiter_requested_at?: string | null; completion_reason?: string | null; @@ -114,6 +115,7 @@ export function applyPairedTaskPatch(args: { finalize_step_done_count?: number; task_done_then_user_reopen_count?: number; empty_step_done_streak?: number; + arbiter_intervention_count?: number; status?: PairedTaskStatus; arbiter_verdict?: string | null; arbiter_requested_at?: string | null; diff --git a/src/types.ts b/src/types.ts index 7c78c22..9ef922c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -132,6 +132,7 @@ export interface PairedTask { finalize_step_done_count?: number | null; task_done_then_user_reopen_count?: number | null; empty_step_done_streak?: number | null; + arbiter_intervention_count?: number | null; status: PairedTaskStatus; arbiter_verdict: string | null; arbiter_requested_at: string | null;