Merge pull request #91 from phj1081/codex/owner/ejclaw

Extract paired verdict signal helpers

src/index.ts

@@ -73,7 +73,7 @@ import {
   isBotMessageSourceKind,
   resolveInjectedMessageSourceKind,
 } from './message-source.js';
-import { parseVisibleVerdict } from './paired-execution-context-shared.js';
+import { parseVisibleVerdict } from './paired-verdict.js';
 
 export function isTerminalStatusMessage(text: string): boolean {
   return parseVisibleVerdict(text) !== 'continue';

src/ipc-outbound-delivery.ts

@@ -1,7 +1,7 @@
 import { createProducedWorkItem } from './db.js';
 import type { WorkItem } from './db/work-items.js';
 import { deliverOpenWorkItem } from './message-runtime-delivery.js';
-import { parseVisibleVerdict } from './paired-execution-context-shared.js';
+import { parseVisibleVerdict } from './paired-verdict.js';
 import { resolveChannelForDeliveryRole } from './router.js';
 import type {
   Channel,

src/message-agent-executor-paired.ts

@@ -14,7 +14,7 @@ import {
   completePairedExecutionContext,
   type PreparedPairedExecutionContext,
 } from './paired-execution-context.js';
-import { parseVisibleVerdict } from './paired-execution-context-shared.js';
+import { parseVisibleVerdict } from './paired-verdict.js';
 import { resolvePairedFollowUpQueueAction } from './message-agent-executor-rules.js';
 import { enqueuePairedFollowUpAfterEvent } from './message-runtime-follow-up.js';
 import type { PairedTurnIdentity } from './paired-turn-identity.js';

src/message-agent-executor-rules.ts

@@ -2,7 +2,7 @@ import {
   resolveFollowUpDispatch,
   resolveNextTurnAction,
 } from './message-runtime-rules.js';
-import { parseVisibleVerdict } from './paired-execution-context-shared.js';
+import { parseVisibleVerdict } from './paired-verdict.js';
 import type { PairedRoomRole, PairedTaskStatus } from './types.js';
 export {
   isRetryableClaudeSessionFailureAttempt,

src/message-runtime-follow-up.ts

@@ -1,6 +1,8 @@
 import { getPairedTurnOutputs } from './db.js';
-import { type VisibleVerdict } from './paired-execution-context-shared.js';
-import { resolveStoredVisibleVerdict } from './paired-verdict.js';
+import {
+  resolveStoredVisibleVerdict,
+  type VisibleVerdict,
+} from './paired-verdict.js';
 import {
   matchesExpectedPairedFollowUpIntent,
   resolveFollowUpDispatch,

src/message-runtime-rules.ts

@@ -4,7 +4,7 @@ import { normalizeStoredSeqCursor } from './message-cursor.js';
 import { isTriggerAllowed, loadSenderAllowlist } from './sender-allowlist.js';
 import { isTaskStatusControlMessage } from './task-watch-status.js';
 import { ARBITER_AGENT_TYPE, REVIEWER_AGENT_TYPE } from './config.js';
-import type { VisibleVerdict } from './paired-execution-context-shared.js';
+import type { VisibleVerdict } from './paired-verdict.js';
 import {
   hasReviewerLease,
   resolveLeaseServiceId,

src/paired-completion-signals.ts

@@ -0,0 +1,109 @@
+import type { VisibleVerdict } from './paired-verdict.js';
+
+export type CompletionSignal =
+  | { kind: 'request_reviewer'; resetStatusToActive: boolean }
+  | { kind: 'request_owner_finalize' }
+  | { kind: 'request_owner_changes' }
+  | { kind: 'request_arbiter' }
+  | { kind: 'complete'; completionReason: 'done' | 'escalated' }
+  | { kind: 'preserve_review_ready' };
+
+export function resolveOwnerCompletionSignal(args: {
+  phase: 'normal' | 'finalize';
+  visibleVerdict: VisibleVerdict;
+  hasChangesSinceApproval?: boolean | null;
+  roundTripCount?: number;
+  deadlockThreshold?: number;
+}): CompletionSignal {
+  const {
+    phase,
+    visibleVerdict,
+    hasChangesSinceApproval = false,
+    roundTripCount = 0,
+    deadlockThreshold = Number.POSITIVE_INFINITY,
+  } = args;
+
+  if (visibleVerdict === 'blocked' || visibleVerdict === 'needs_context') {
+    return { kind: 'request_arbiter' };
+  }
+
+  if (phase === 'normal') {
+    return {
+      kind: 'request_reviewer',
+      resetStatusToActive: false,
+    };
+  }
+
+  if (visibleVerdict === 'step_done') {
+    return {
+      kind: 'request_reviewer',
+      resetStatusToActive: true,
+    };
+  }
+
+  const needsReReview =
+    visibleVerdict === 'done_with_concerns' || hasChangesSinceApproval === true;
+
+  if (needsReReview) {
+    if (roundTripCount >= deadlockThreshold) {
+      return { kind: 'request_arbiter' };
+    }
+    return {
+      kind: 'request_reviewer',
+      resetStatusToActive: true,
+    };
+  }
+
+  return {
+    kind: 'complete',
+    completionReason: 'done',
+  };
+}
+
+export function resolveReviewerCompletionSignal(args: {
+  visibleVerdict: VisibleVerdict;
+  roundTripCount: number;
+  deadlockThreshold: number;
+}): CompletionSignal {
+  const { visibleVerdict, roundTripCount, deadlockThreshold } = args;
+
+  switch (visibleVerdict) {
+    case 'task_done':
+    case 'done':
+      return { kind: 'request_owner_finalize' };
+    case 'step_done':
+      return { kind: 'request_owner_changes' };
+    case 'blocked':
+    case 'needs_context':
+      return { kind: 'request_arbiter' };
+    case 'done_with_concerns':
+    case 'continue':
+    default:
+      if (roundTripCount >= deadlockThreshold) {
+        return { kind: 'request_arbiter' };
+      }
+      return { kind: 'request_owner_changes' };
+  }
+}
+
+export function resolveReviewerFailureSignal(args: {
+  visibleVerdict: VisibleVerdict;
+}): CompletionSignal {
+  const { visibleVerdict } = args;
+
+  switch (visibleVerdict) {
+    case 'task_done':
+    case 'done':
+      return { kind: 'request_owner_finalize' };
+    case 'blocked':
+    case 'needs_context':
+      return {
+        kind: 'complete',
+        completionReason: 'escalated',
+      };
+    case 'done_with_concerns':
+    case 'continue':
+    default:
+      return { kind: 'preserve_review_ready' };
+  }
+}

src/paired-execution-context-arbiter.ts

@@ -1,9 +1,7 @@
 import { ARBITER_DEADLOCK_THRESHOLD } from './config.js';
 import { logger } from './logger.js';
-import {
-  classifyArbiterVerdict,
-  transitionPairedTaskStatus,
-} from './paired-execution-context-shared.js';
+import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
+import { classifyArbiterVerdict } from './paired-verdict.js';
 import type { PairedTask } from './types.js';
 
 export function handleFailedArbiterExecution(args: {

src/paired-execution-context-owner.ts

@@ -12,12 +12,12 @@ import { markPairedTaskReviewReady } from './paired-workspace-manager.js';
 import {
   applyPairedTaskPatch,
   hasCodeChangesSinceRef,
-  parseVisibleVerdict,
   requestArbiterOrEscalate,
-  resolveOwnerCompletionSignal,
   resolveCanonicalSourceRef,
   transitionPairedTaskStatus,
 } from './paired-execution-context-shared.js';
+import { resolveOwnerCompletionSignal } from './paired-completion-signals.js';
+import { parseVisibleVerdict } from './paired-verdict.js';
 import type { PairedTask } from './types.js';
 
 type OwnerFinalizeOutcome = 'stop' | 're_review';

src/paired-execution-context-reviewer.ts

@@ -2,13 +2,15 @@ import { ARBITER_DEADLOCK_THRESHOLD } from './config.js';
 import { getPairedWorkspace } from './db.js';
 import { logger } from './logger.js';
 import {
-  parseVisibleVerdict,
   requestArbiterOrEscalate,
-  resolveReviewerCompletionSignal,
-  resolveReviewerFailureSignal,
   resolveCanonicalSourceRef,
   transitionPairedTaskStatus,
 } from './paired-execution-context-shared.js';
+import {
+  resolveReviewerCompletionSignal,
+  resolveReviewerFailureSignal,
+} from './paired-completion-signals.js';
+import { parseVisibleVerdict } from './paired-verdict.js';
 import type { PairedTask } from './types.js';
 
 export function handleFailedReviewerExecution(args: {

src/paired-execution-context-shared.test.ts

@@ -1,25 +1,12 @@
 import { describe, expect, it } from 'vitest';
 
 import {
-  parseVisibleVerdict,
   resolveOwnerCompletionSignal,
   resolveReviewerCompletionSignal,
   resolveReviewerFailureSignal,
-} from './paired-execution-context-shared.js';
+} from './paired-completion-signals.js';
 
 describe('paired execution context shared verdict helpers', () => {
-  it('parses visible verdicts from the first summary line only', () => {
-    expect(parseVisibleVerdict('STEP_DONE\nmore to do')).toBe('step_done');
-    expect(parseVisibleVerdict('TASK_DONE\nall done')).toBe('task_done');
-    expect(
-      parseVisibleVerdict(
-        'DONE_WITH_CONCERNS\n\nfollow-up detail that should not affect parsing',
-      ),
-    ).toBe('done_with_concerns');
-    expect(parseVisibleVerdict('BLOCKED\nextra detail')).toBe('blocked');
-    expect(parseVisibleVerdict('random prose')).toBe('continue');
-  });
-
   it('maps normal owner completion verdicts to reviewer or arbiter signals', () => {
     expect(
       resolveOwnerCompletionSignal({

src/paired-execution-context-shared.ts

@@ -3,142 +3,19 @@ import { execFileSync } from 'child_process';
 import { isArbiterEnabled } from './config.js';
 import { updatePairedTaskIfUnchanged } from './db.js';
 import { logger } from './logger.js';
-import { parseVisibleVerdict, type VisibleVerdict } from './paired-verdict.js';
 import type { PairedTaskStatus } from './types.js';
 
-export type CompletionSignal =
-  | { kind: 'request_reviewer'; resetStatusToActive: boolean }
-  | { kind: 'request_owner_finalize' }
-  | { kind: 'request_owner_changes' }
-  | { kind: 'request_arbiter' }
-  | { kind: 'complete'; completionReason: 'done' | 'escalated' }
-  | { kind: 'preserve_review_ready' };
-
-export { parseVisibleVerdict };
-export type { VisibleVerdict };
-
-export function resolveOwnerCompletionSignal(args: {
-  phase: 'normal' | 'finalize';
-  visibleVerdict: VisibleVerdict;
-  hasChangesSinceApproval?: boolean | null;
-  roundTripCount?: number;
-  deadlockThreshold?: number;
-}): CompletionSignal {
-  const {
-    phase,
-    visibleVerdict,
-    hasChangesSinceApproval = false,
-    roundTripCount = 0,
-    deadlockThreshold = Number.POSITIVE_INFINITY,
-  } = args;
-
-  if (visibleVerdict === 'blocked' || visibleVerdict === 'needs_context') {
-    return { kind: 'request_arbiter' };
-  }
-
-  if (phase === 'normal') {
-    return {
-      kind: 'request_reviewer',
-      resetStatusToActive: false,
-    };
-  }
-
-  if (visibleVerdict === 'step_done') {
-    return {
-      kind: 'request_reviewer',
-      resetStatusToActive: true,
-    };
-  }
-
-  const needsReReview =
-    visibleVerdict === 'done_with_concerns' || hasChangesSinceApproval === true;
-
-  if (needsReReview) {
-    if (roundTripCount >= deadlockThreshold) {
-      return { kind: 'request_arbiter' };
-    }
-    return {
-      kind: 'request_reviewer',
-      resetStatusToActive: true,
-    };
-  }
-
-  return {
-    kind: 'complete',
-    completionReason: 'done',
-  };
-}
-
-export function resolveReviewerCompletionSignal(args: {
-  visibleVerdict: VisibleVerdict;
-  roundTripCount: number;
-  deadlockThreshold: number;
-}): CompletionSignal {
-  const { visibleVerdict, roundTripCount, deadlockThreshold } = args;
-
-  switch (visibleVerdict) {
-    case 'task_done':
-    case 'done':
-      return { kind: 'request_owner_finalize' };
-    case 'step_done':
-      return { kind: 'request_owner_changes' };
-    case 'blocked':
-    case 'needs_context':
-      return { kind: 'request_arbiter' };
-    case 'done_with_concerns':
-    case 'continue':
-    default:
-      if (roundTripCount >= deadlockThreshold) {
-        return { kind: 'request_arbiter' };
-      }
-      return { kind: 'request_owner_changes' };
-  }
-}
-
-export function resolveReviewerFailureSignal(args: {
-  visibleVerdict: VisibleVerdict;
-}): CompletionSignal {
-  const { visibleVerdict } = args;
-
-  switch (visibleVerdict) {
-    case 'task_done':
-    case 'done':
-      return { kind: 'request_owner_finalize' };
-    case 'blocked':
-    case 'needs_context':
-      return {
-        kind: 'complete',
-        completionReason: 'escalated',
-      };
-    case 'done_with_concerns':
-    case 'continue':
-    default:
-      return { kind: 'preserve_review_ready' };
-  }
-}
-
-export type ArbiterVerdictResult =
-  | 'proceed'
-  | 'revise'
-  | 'reset'
-  | 'escalate'
-  | 'unknown';
-
-export 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();
-  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';
-}
+export {
+  resolveOwnerCompletionSignal,
+  resolveReviewerCompletionSignal,
+  resolveReviewerFailureSignal,
+} from './paired-completion-signals.js';
+export type { CompletionSignal } from './paired-completion-signals.js';
+export {
+  classifyArbiterVerdict,
+  parseVisibleVerdict,
+} from './paired-verdict.js';
+export type { ArbiterVerdictResult, VisibleVerdict } from './paired-verdict.js';
 
 export function resolveCanonicalSourceRef(workDir: string): string {
   const treeHash = resolveCanonicalTreeHash(workDir);

src/paired-verdict.test.ts

@@ -0,0 +1,34 @@
+import { describe, expect, it } from 'vitest';
+
+import {
+  classifyArbiterVerdict,
+  parseVisibleVerdict,
+} from './paired-verdict.js';
+
+describe('paired verdict parser', () => {
+  it('parses visible verdicts from the first summary line only', () => {
+    expect(parseVisibleVerdict('STEP_DONE\nmore to do')).toBe('step_done');
+    expect(parseVisibleVerdict('TASK_DONE\nall done')).toBe('task_done');
+    expect(
+      parseVisibleVerdict(
+        'DONE_WITH_CONCERNS\n\nfollow-up detail that should not affect parsing',
+      ),
+    ).toBe('done_with_concerns');
+    expect(parseVisibleVerdict('BLOCKED\nextra detail')).toBe('blocked');
+    expect(parseVisibleVerdict('random prose')).toBe('continue');
+  });
+
+  it('classifies arbiter verdicts from the first visible line', () => {
+    expect(classifyArbiterVerdict('PROCEED\ncontinue')).toBe('proceed');
+    expect(classifyArbiterVerdict('**VERDICT: REVISE**\nfix it')).toBe(
+      'revise',
+    );
+    expect(
+      classifyArbiterVerdict(
+        '<internal>private notes</internal>\nVERDICT — RESET\nrestart',
+      ),
+    ).toBe('reset');
+    expect(classifyArbiterVerdict('ESCALATE\nblocked')).toBe('escalate');
+    expect(classifyArbiterVerdict('no verdict here')).toBe('unknown');
+  });
+});

src/paired-verdict.ts

@@ -7,6 +7,13 @@ export type VisibleVerdict =
   | 'needs_context'
   | 'continue';
 
+export type ArbiterVerdictResult =
+  | 'proceed'
+  | 'revise'
+  | 'reset'
+  | 'escalate'
+  | 'unknown';
+
 export function parseVisibleVerdict(
   summary: string | null | undefined,
 ): VisibleVerdict {
@@ -26,6 +33,22 @@ export function parseVisibleVerdict(
   return 'continue';
 }
 
+export 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();
+  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';
+}
+
 export function resolveStoredVisibleVerdict(args: {
   verdict?: VisibleVerdict | null;
   outputText?: string | null;