Refactor paired execution flow boundaries

src/paired-execution-context.ts

@@ -1,4 +1,3 @@
-import { execFileSync } from 'child_process';
 import crypto from 'crypto';
 import fs from 'fs';
 import path from 'path';
@@ -12,7 +11,6 @@ import {
   DATA_DIR,
   PAIRED_MAX_ROUND_TRIPS,
   REVIEWER_AGENT_TYPE,
-  isArbiterEnabled,
 } from './config.js';
 import {
   createPairedTask,
@@ -25,6 +23,22 @@ import {
   upsertPairedProject,
 } from './db.js';
 import { logger } from './logger.js';
+import {
+  handleArbiterCompletion,
+  handleFailedArbiterExecution,
+} from './paired-execution-context-arbiter.js';
+import {
+  handleFailedOwnerExecution,
+  handleOwnerCompletion,
+} from './paired-execution-context-owner.js';
+import {
+  handleFailedReviewerExecution,
+  handleReviewerCompletion,
+} from './paired-execution-context-reviewer.js';
+import {
+  resolveCanonicalSourceRef,
+  requestArbiterOrEscalate,
+} from './paired-execution-context-shared.js';
 import {
   markPairedTaskReviewReady,
   prepareReviewerWorkspaceForExecution,
@@ -40,137 +54,6 @@ import type {
 } from './types.js';
 import { resolveRoleAgentPlan } from './role-agent-plan.js';
 
-// ---------------------------------------------------------------------------
-// Reviewer verdict detection
-// ---------------------------------------------------------------------------
-
-type Verdict =
-  | 'done'
-  | 'done_with_concerns'
-  | 'blocked'
-  | 'needs_context'
-  | 'continue';
-
-function classifyVerdict(summary: string | null | undefined): Verdict {
-  if (!summary) return 'continue';
-  // Strip <internal>...</internal> tags — these are internal reasoning, not the verdict
-  const cleaned = summary.replace(/<internal>[\s\S]*?<\/internal>/g, '').trim();
-  if (!cleaned) return 'continue';
-  const firstLine = cleaned.split('\n')[0].trim();
-  // Match verdict markers at the start of the first visible line
-  if (/^\*{0,2}BLOCKED\*{0,2}\b/i.test(firstLine)) return 'blocked';
-  if (/^\*{0,2}NEEDS_CONTEXT\*{0,2}\b/i.test(firstLine)) return 'needs_context';
-  if (/^\*{0,2}DONE_WITH_CONCERNS\*{0,2}\b/i.test(firstLine))
-    return 'done_with_concerns';
-  if (/^\*{0,2}DONE\*{0,2}\b/i.test(firstLine)) return 'done';
-  if (/^\*{0,2}Approved\.?\*{0,2}/i.test(firstLine)) return 'done';
-  if (/^\*{0,2}LGTM\*{0,2}/i.test(firstLine)) return 'done';
-  return 'continue';
-}
-
-// ---------------------------------------------------------------------------
-// Arbiter verdict detection
-// ---------------------------------------------------------------------------
-
-type ArbiterVerdictResult =
-  | 'proceed'
-  | 'revise'
-  | 'reset'
-  | 'escalate'
-  | 'unknown';
-
-function classifyArbiterVerdict(
-  summary: string | null | undefined,
-): ArbiterVerdictResult {
-  if (!summary) return 'unknown';
-  const cleaned = summary.replace(/<internal>[\s\S]*?<\/internal>/g, '').trim();
-  if (!cleaned) return 'unknown';
-  const firstLine = cleaned.split('\n')[0].trim();
-  // Match verdict keywords with optional prefix like "VERDICT:" and markdown bold
-  const verdictMatch = firstLine.match(
-    /\*{0,2}(?:VERDICT\s*[:—-]\s*)?(PROCEED|REVISE|RESET|ESCALATE)\*{0,2}/i,
-  );
-  if (verdictMatch) {
-    return verdictMatch[1].toLowerCase() as ArbiterVerdictResult;
-  }
-  return 'unknown';
-}
-
-// ---------------------------------------------------------------------------
-// Helpers
-// ---------------------------------------------------------------------------
-
-function resolveCanonicalSourceRef(workDir: string): string {
-  const treeHash = resolveCanonicalTreeHash(workDir);
-  return treeHash || 'HEAD';
-}
-
-function resolveCanonicalTreeHash(workDir: string): string | null {
-  try {
-    const treeHash = execFileSync('git', ['rev-parse', 'HEAD^{tree}'], {
-      cwd: workDir,
-      encoding: 'utf-8',
-      stdio: ['ignore', 'pipe', 'pipe'],
-    }).trim();
-    return treeHash || null;
-  } catch {
-    return null;
-  }
-}
-
-function hasCodeChangesSinceRef(
-  workDir: string,
-  sourceRef: string | null | undefined,
-): boolean | null {
-  if (!sourceRef) return null;
-  try {
-    execFileSync('git', ['diff', '--quiet', sourceRef, 'HEAD'], {
-      cwd: workDir,
-      stdio: ['ignore', 'pipe', 'pipe'],
-    });
-    return false;
-  } catch (error) {
-    const exitCode =
-      typeof error === 'object' &&
-      error !== null &&
-      'status' in error &&
-      typeof (error as { status?: unknown }).status === 'number'
-        ? (error as { status: number }).status
-        : null;
-    if (exitCode === 1) {
-      return true;
-    }
-    return null;
-  }
-}
-
-function requestArbiterOrEscalate(args: {
-  taskId: string;
-  now: string;
-  arbiterLogMessage: string;
-  escalateLogMessage: string;
-  logContext?: Record<string, unknown>;
-}): void {
-  const { taskId, now, arbiterLogMessage, escalateLogMessage, logContext } =
-    args;
-  if (isArbiterEnabled()) {
-    updatePairedTask(taskId, {
-      status: 'arbiter_requested',
-      arbiter_requested_at: now,
-      updated_at: now,
-    });
-    logger.info(logContext ?? { taskId }, arbiterLogMessage);
-    return;
-  }
-
-  updatePairedTask(taskId, {
-    status: 'completed',
-    completion_reason: 'escalated',
-    updated_at: now,
-  });
-  logger.info(logContext ?? { taskId }, escalateLogMessage);
-}
-
 function ensurePairedProject(
   group: RegisteredGroup,
   chatJid: string,
@@ -444,399 +327,6 @@ export function preparePairedExecutionContext(args: {
   };
 }
 
-// ---------------------------------------------------------------------------
-// completePairedExecutionContext
-// ---------------------------------------------------------------------------
-
-function handleFailedReviewerExecution(args: {
-  task: PairedTask;
-  taskId: string;
-  summary?: string | null;
-}): void {
-  const { task, taskId, summary } = args;
-  const now = new Date().toISOString();
-
-  if (summary) {
-    const verdict = classifyVerdict(summary);
-    if (
-      verdict === 'done' ||
-      verdict === 'blocked' ||
-      verdict === 'needs_context'
-    ) {
-      const ownerWs =
-        verdict === 'done' ? getPairedWorkspace(taskId, 'owner') : null;
-      const approvedSourceRef =
-        verdict === 'done' && ownerWs?.workspace_dir
-          ? resolveCanonicalSourceRef(ownerWs.workspace_dir)
-          : task.source_ref;
-      updatePairedTask(taskId, {
-        status: verdict === 'done' ? 'merge_ready' : 'completed',
-        ...(verdict === 'done' ? { source_ref: approvedSourceRef } : {}),
-        ...(verdict !== 'done' ? { completion_reason: 'escalated' } : {}),
-        updated_at: now,
-      });
-      logger.info(
-        {
-          taskId,
-          verdict,
-          approvedSourceRef,
-          summary: summary.slice(0, 100),
-        },
-        'Reviewer verdict detected from failed execution — stopping ping-pong',
-      );
-      return;
-    }
-  }
-
-  const fallbackStatus =
-    task.status === 'in_review' || task.status === 'review_ready'
-      ? 'review_ready'
-      : task.status;
-  if (fallbackStatus !== task.status) {
-    updatePairedTask(taskId, { status: fallbackStatus, updated_at: now });
-    logger.warn(
-      {
-        taskId,
-        role: 'reviewer',
-        previousStatus: task.status,
-        nextStatus: fallbackStatus,
-      },
-      'Preserved reviewer task in review-ready state after failed execution',
-    );
-  }
-}
-
-function handleFailedArbiterExecution(args: {
-  task: PairedTask;
-  taskId: string;
-}): void {
-  const { task, taskId } = args;
-  const now = new Date().toISOString();
-  const fallbackStatus =
-    task.status === 'in_arbitration' || task.status === 'arbiter_requested'
-      ? 'arbiter_requested'
-      : task.status;
-  if (fallbackStatus !== task.status) {
-    updatePairedTask(taskId, { status: fallbackStatus, updated_at: now });
-    logger.warn(
-      {
-        taskId,
-        role: 'arbiter',
-        previousStatus: task.status,
-        nextStatus: fallbackStatus,
-      },
-      'Preserved arbiter task in arbitration-requested state after failed execution',
-    );
-  }
-}
-
-function handleFailedOwnerExecution(args: {
-  task: PairedTask;
-  taskId: string;
-}): void {
-  const { task, taskId } = args;
-  if (task.status !== 'active') {
-    const now = new Date().toISOString();
-    updatePairedTask(taskId, { status: 'active', updated_at: now });
-    logger.info(
-      { taskId, role: 'owner', previousStatus: task.status },
-      'Reset task to active after failed execution',
-    );
-  }
-}
-
-function handleOwnerFinalizeCompletion(args: {
-  task: PairedTask;
-  taskId: string;
-  summary?: string | null;
-  now: string;
-}): boolean {
-  const { task, taskId, summary, now } = args;
-  const ownerVerdict = classifyVerdict(summary);
-
-  if (ownerVerdict === 'blocked' || ownerVerdict === 'needs_context') {
-    requestArbiterOrEscalate({
-      taskId,
-      now,
-      arbiterLogMessage: 'Owner blocked during finalize — requesting arbiter',
-      escalateLogMessage: 'Owner blocked during finalize — escalating to user',
-      logContext: {
-        taskId,
-        ownerVerdict,
-        summary: summary?.slice(0, 100),
-      },
-    });
-    return true;
-  }
-
-  if (ownerVerdict === 'done_with_concerns') {
-    if (task.round_trip_count >= ARBITER_DEADLOCK_THRESHOLD) {
-      requestArbiterOrEscalate({
-        taskId,
-        now,
-        arbiterLogMessage: 'Owner finalize loop detected — requesting arbiter',
-        escalateLogMessage: 'Owner finalize loop detected — escalating to user',
-        logContext: {
-          taskId,
-          ownerVerdict,
-          roundTrips: task.round_trip_count,
-        },
-      });
-      return true;
-    }
-    updatePairedTask(taskId, { status: 'active', updated_at: now });
-    logger.info(
-      {
-        taskId,
-        ownerVerdict,
-        summary: summary?.slice(0, 100),
-      },
-      'Owner raised concerns during finalize — task set back to active',
-    );
-    return false;
-  }
-
-  const workspace = getPairedWorkspace(task.id, 'owner');
-  const hasNewChanges = workspace?.workspace_dir
-    ? hasCodeChangesSinceRef(workspace.workspace_dir, task.source_ref)
-    : null;
-
-  if (hasNewChanges === true) {
-    if (task.round_trip_count >= ARBITER_DEADLOCK_THRESHOLD) {
-      requestArbiterOrEscalate({
-        taskId,
-        now,
-        arbiterLogMessage:
-          'Owner finalize DONE loop detected — requesting arbiter',
-        escalateLogMessage:
-          'Owner finalize DONE loop detected — escalating to user',
-        logContext: {
-          taskId,
-          roundTrips: task.round_trip_count,
-          hasNewChanges,
-        },
-      });
-      return true;
-    }
-    logger.info(
-      {
-        taskId,
-        sourceRef: task.source_ref,
-        hasNewChanges,
-      },
-      'Owner made changes after reviewer approval — re-triggering review',
-    );
-    return false;
-  }
-
-  updatePairedTask(taskId, {
-    status: 'completed',
-    completion_reason: 'done',
-    updated_at: now,
-  });
-  logger.info(
-    { taskId, hasNewChanges, summary: summary?.slice(0, 100) },
-    'Owner finalized after reviewer approval — task completed',
-  );
-  return true;
-}
-
-function handleOwnerCompletion(args: {
-  task: PairedTask;
-  taskId: string;
-  summary?: string | null;
-}): void {
-  const { task, taskId, summary } = args;
-  const now = new Date().toISOString();
-
-  if (task.status === 'merge_ready') {
-    const shouldStop = handleOwnerFinalizeCompletion({
-      task,
-      taskId,
-      summary,
-      now,
-    });
-    if (shouldStop) {
-      return;
-    }
-  }
-
-  if (hasActiveCiWatcherForChat(task.chat_jid)) {
-    logger.info(
-      { taskId, chatJid: task.chat_jid },
-      'Active CI watcher found, deferring auto-review until watcher completes',
-    );
-    return;
-  }
-
-  if (task.status !== 'merge_ready') {
-    const ownerVerdict = classifyVerdict(summary);
-    if (ownerVerdict === 'blocked' || ownerVerdict === 'needs_context') {
-      requestArbiterOrEscalate({
-        taskId,
-        now,
-        arbiterLogMessage: 'Owner blocked/needs_context — requesting arbiter',
-        escalateLogMessage: 'Owner blocked/needs_context — escalating to user',
-        logContext: {
-          taskId,
-          ownerVerdict,
-          summary: summary?.slice(0, 100),
-        },
-      });
-      return;
-    }
-  }
-
-  if (task.round_trip_count >= PAIRED_MAX_ROUND_TRIPS) {
-    logger.info(
-      {
-        taskId,
-        roundTrips: task.round_trip_count,
-        max: PAIRED_MAX_ROUND_TRIPS,
-      },
-      'Round trip limit reached, skipping auto-review',
-    );
-    return;
-  }
-
-  const result = markPairedTaskReviewReady(taskId);
-  if (result) {
-    updatePairedTask(taskId, {
-      round_trip_count: task.round_trip_count + 1,
-      review_requested_at: now,
-      updated_at: now,
-    });
-    logger.info(
-      { taskId, roundTrip: task.round_trip_count + 1 },
-      'Auto-triggered reviewer after owner completion',
-    );
-  }
-}
-
-function handleReviewerCompletion(args: {
-  task: PairedTask;
-  taskId: string;
-  summary?: string | null;
-}): void {
-  const { task, taskId, summary } = args;
-  const now = new Date().toISOString();
-  const verdict = classifyVerdict(summary);
-
-  switch (verdict) {
-    case 'done': {
-      const ownerWs = getPairedWorkspace(taskId, 'owner');
-      const approvedSourceRef = ownerWs?.workspace_dir
-        ? resolveCanonicalSourceRef(ownerWs.workspace_dir)
-        : task.source_ref;
-      updatePairedTask(taskId, {
-        status: 'merge_ready',
-        source_ref: approvedSourceRef,
-        updated_at: now,
-      });
-      logger.info(
-        {
-          taskId,
-          verdict,
-          approvedSourceRef,
-          summary: summary?.slice(0, 100),
-        },
-        'Reviewer approved — owner gets final turn to finalize',
-      );
-      return;
-    }
-
-    case 'blocked':
-    case 'needs_context':
-      requestArbiterOrEscalate({
-        taskId,
-        now,
-        arbiterLogMessage:
-          'Reviewer blocked/needs_context — requesting arbiter before escalating',
-        escalateLogMessage: 'Reviewer escalated to user — ping-pong stopped',
-        logContext: { taskId, verdict, summary: summary?.slice(0, 100) },
-      });
-      return;
-
-    case 'done_with_concerns':
-    case 'continue':
-    default:
-      if (task.round_trip_count >= ARBITER_DEADLOCK_THRESHOLD) {
-        requestArbiterOrEscalate({
-          taskId,
-          now,
-          arbiterLogMessage:
-            'Deadlock detected — requesting arbiter intervention',
-          escalateLogMessage:
-            'Stopped ping-pong — escalating to user (arbiter not configured)',
-          logContext: {
-            taskId,
-            verdict,
-            roundTrips: task.round_trip_count,
-          },
-        });
-        return;
-      }
-      updatePairedTask(taskId, { status: 'active', updated_at: now });
-      logger.info(
-        { taskId, verdict },
-        'Reviewer has feedback, task set back to active for owner',
-      );
-      return;
-  }
-}
-
-function handleArbiterCompletion(args: {
-  taskId: string;
-  summary?: string | null;
-}): void {
-  const { taskId, summary } = args;
-  const now = new Date().toISOString();
-  const arbiterVerdict = classifyArbiterVerdict(summary);
-
-  logger.info(
-    { taskId, arbiterVerdict, summary: summary?.slice(0, 200) },
-    'Arbiter verdict rendered',
-  );
-
-  switch (arbiterVerdict) {
-    case 'proceed':
-    case 'revise':
-    case 'reset':
-      updatePairedTask(taskId, {
-        status: 'active',
-        round_trip_count: Math.max(0, ARBITER_DEADLOCK_THRESHOLD - 1),
-        arbiter_verdict: arbiterVerdict,
-        updated_at: now,
-      });
-      logger.info(
-        { taskId, arbiterVerdict },
-        'Arbiter resolved deadlock — resuming ping-pong',
-      );
-      return;
-    case 'escalate':
-      updatePairedTask(taskId, {
-        status: 'completed',
-        arbiter_verdict: 'escalate',
-        completion_reason: 'arbiter_escalated',
-        updated_at: now,
-      });
-      logger.info({ taskId }, 'Arbiter escalated to user — task completed');
-      return;
-    default:
-      updatePairedTask(taskId, {
-        status: 'active',
-        round_trip_count: Math.max(0, ARBITER_DEADLOCK_THRESHOLD - 1),
-        arbiter_verdict: 'unknown',
-        updated_at: now,
-      });
-      logger.warn(
-        { taskId, summary: summary?.slice(0, 200) },
-        'Arbiter verdict unrecognized — falling back to proceed',
-      );
-      return;
-  }
-}
-
 export function completePairedExecutionContext(args: {
   taskId: string;
   role: PairedRoomRole;