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

Extract paired task status helpers

src/message-runtime.ts

@@ -48,7 +48,7 @@ import {
   schedulePairedFollowUpWithMessageCheck,
 } from './message-runtime-follow-up.js';
 import { type ScheduledPairedFollowUpIntentKind } from './paired-follow-up-scheduler.js';
-import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
+import { transitionPairedTaskStatus } from './paired-task-status.js';
 import {
   Channel,
   NewMessage,

src/paired-arbiter-request.ts

@@ -1,6 +1,6 @@
 import { isArbiterEnabled } from './config.js';
 import { logger } from './logger.js';
-import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
+import { transitionPairedTaskStatus } from './paired-task-status.js';
 import type { PairedTaskStatus } from './types.js';
 
 export function requestArbiterOrEscalate(args: {

src/paired-execution-context-arbiter.ts

@@ -1,6 +1,6 @@
 import { ARBITER_DEADLOCK_THRESHOLD } from './config.js';
 import { logger } from './logger.js';
-import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
+import { transitionPairedTaskStatus } from './paired-task-status.js';
 import { classifyArbiterVerdict } from './paired-verdict.js';
 import type { PairedTask } from './types.js';
 

src/paired-execution-context-owner.ts

@@ -13,7 +13,7 @@ import { requestArbiterOrEscalate } from './paired-arbiter-request.js';
 import {
   applyPairedTaskPatch,
   transitionPairedTaskStatus,
-} from './paired-execution-context-shared.js';
+} from './paired-task-status.js';
 import { resolveOwnerCompletionSignal } from './paired-completion-signals.js';
 import {
   hasCodeChangesSinceRef,

src/paired-execution-context-reviewer.ts

@@ -2,7 +2,7 @@ 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-execution-context-shared.js';
+import { transitionPairedTaskStatus } from './paired-task-status.js';
 import {
   resolveReviewerCompletionSignal,
   resolveReviewerFailureSignal,

src/paired-execution-context-shared.ts

@@ -1,7 +1,3 @@
-import { updatePairedTaskIfUnchanged } from './db.js';
-import { logger } from './logger.js';
-import type { PairedTaskStatus } from './types.js';
-
 export { requestArbiterOrEscalate } from './paired-arbiter-request.js';
 export type { RequestArbiterOrEscalateFn } from './paired-arbiter-request.js';
 export {
@@ -19,139 +15,13 @@ export {
   hasCodeChangesSinceRef,
   resolveCanonicalSourceRef,
 } from './paired-source-ref.js';
-
+export {
-const ALLOWED_PAIRED_STATUS_TRANSITIONS: Record<
+  ALLOWED_PAIRED_STATUS_TRANSITIONS,
-  PairedTaskStatus,
+  applyPairedTaskPatch,
-  ReadonlySet<PairedTaskStatus>
+  assertPairedTaskStatusTransition,
-> = {
+  transitionPairedTaskStatus,
-  active: new Set(['review_ready', 'arbiter_requested', 'completed']),
+} from './paired-task-status.js';
-  review_ready: new Set([
+export type {
-    'active',
+  ApplyPairedTaskPatchFn,
-    'in_review',
+  TransitionPairedTaskStatusFn,
-    'arbiter_requested',
+} from './paired-task-status.js';
-    'completed',
-  ]),
-  in_review: new Set([
-    'active',
-    'review_ready',
-    'merge_ready',
-    'arbiter_requested',
-    'completed',
-  ]),
-  merge_ready: new Set(['active', 'arbiter_requested', 'completed']),
-  completed: new Set(),
-  arbiter_requested: new Set(['in_arbitration', 'completed']),
-  in_arbitration: new Set(['active', 'arbiter_requested', 'completed']),
-};
-
-export function assertPairedTaskStatusTransition(args: {
-  currentStatus: PairedTaskStatus;
-  nextStatus: PairedTaskStatus;
-}): void {
-  const { currentStatus, nextStatus } = args;
-  if (currentStatus === nextStatus) {
-    return;
-  }
-
-  if (ALLOWED_PAIRED_STATUS_TRANSITIONS[currentStatus].has(nextStatus)) {
-    return;
-  }
-
-  throw new Error(
-    `Invalid paired task status transition: ${currentStatus} -> ${nextStatus}`,
-  );
-}
-
-export function transitionPairedTaskStatus(args: {
-  taskId: string;
-  currentStatus: PairedTaskStatus;
-  nextStatus: PairedTaskStatus;
-  expectedUpdatedAt: string;
-  updatedAt: string;
-  patch?: {
-    title?: string | null;
-    source_ref?: string | null;
-    plan_notes?: string | null;
-    review_requested_at?: string | null;
-    round_trip_count?: number;
-    owner_failure_count?: number;
-    owner_step_done_streak?: number;
-    finalize_step_done_count?: number;
-    task_done_then_user_reopen_count?: number;
-    empty_step_done_streak?: number;
-    arbiter_verdict?: string | null;
-    arbiter_requested_at?: string | null;
-    completion_reason?: string | null;
-  };
-}): boolean {
-  assertPairedTaskStatusTransition({
-    currentStatus: args.currentStatus,
-    nextStatus: args.nextStatus,
-  });
-
-  const updated = updatePairedTaskIfUnchanged(
-    args.taskId,
-    args.expectedUpdatedAt,
-    {
-      ...args.patch,
-      status: args.nextStatus,
-      updated_at: args.updatedAt,
-    },
-  );
-  if (!updated) {
-    logger.warn(
-      {
-        taskId: args.taskId,
-        currentStatus: args.currentStatus,
-        nextStatus: args.nextStatus,
-        expectedUpdatedAt: args.expectedUpdatedAt,
-      },
-      'Skipped stale paired task status transition because the task revision changed',
-    );
-  }
-  return updated;
-}
-
-export function applyPairedTaskPatch(args: {
-  taskId: string;
-  expectedUpdatedAt: string;
-  updatedAt: string;
-  patch: {
-    title?: string | null;
-    source_ref?: string | null;
-    plan_notes?: string | null;
-    review_requested_at?: string | null;
-    round_trip_count?: number;
-    owner_failure_count?: number;
-    owner_step_done_streak?: number;
-    finalize_step_done_count?: number;
-    task_done_then_user_reopen_count?: number;
-    empty_step_done_streak?: number;
-    status?: PairedTaskStatus;
-    arbiter_verdict?: string | null;
-    arbiter_requested_at?: string | null;
-    completion_reason?: string | null;
-  };
-}): boolean {
-  const updated = updatePairedTaskIfUnchanged(
-    args.taskId,
-    args.expectedUpdatedAt,
-    {
-      ...args.patch,
-      updated_at: args.updatedAt,
-    },
-  );
-  if (!updated) {
-    logger.warn(
-      {
-        taskId: args.taskId,
-        expectedUpdatedAt: args.expectedUpdatedAt,
-      },
-      'Skipped stale paired task patch because the task revision changed',
-    );
-  }
-  return updated;
-}
-
-export type TransitionPairedTaskStatusFn = typeof transitionPairedTaskStatus;
-export type ApplyPairedTaskPatchFn = typeof applyPairedTaskPatch;

src/paired-execution-context.ts

@@ -49,7 +49,7 @@ import {
 import {
   applyPairedTaskPatch,
   transitionPairedTaskStatus,
-} from './paired-execution-context-shared.js';
+} from './paired-task-status.js';
 import { requestArbiterOrEscalate } from './paired-arbiter-request.js';
 import { resolveCanonicalSourceRef } from './paired-source-ref.js';
 import {

src/paired-task-status.test.ts

@@ -0,0 +1,118 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+vi.mock('./db.js', () => ({
+  updatePairedTaskIfUnchanged: vi.fn(() => true),
+}));
+
+vi.mock('./logger.js', () => ({
+  logger: {
+    warn: vi.fn(),
+  },
+}));
+
+import { updatePairedTaskIfUnchanged } from './db.js';
+import { logger } from './logger.js';
+import {
+  applyPairedTaskPatch,
+  assertPairedTaskStatusTransition,
+  transitionPairedTaskStatus,
+} from './paired-task-status.js';
+
+describe('paired task status transitions', () => {
+  beforeEach(() => {
+    vi.mocked(updatePairedTaskIfUnchanged).mockReset();
+    vi.mocked(updatePairedTaskIfUnchanged).mockReturnValue(true);
+    vi.mocked(logger.warn).mockReset();
+  });
+
+  it('allows valid transitions and same-status no-ops', () => {
+    expect(() =>
+      assertPairedTaskStatusTransition({
+        currentStatus: 'active',
+        nextStatus: 'review_ready',
+      }),
+    ).not.toThrow();
+    expect(() =>
+      assertPairedTaskStatusTransition({
+        currentStatus: 'review_ready',
+        nextStatus: 'review_ready',
+      }),
+    ).not.toThrow();
+  });
+
+  it('rejects invalid transitions', () => {
+    expect(() =>
+      assertPairedTaskStatusTransition({
+        currentStatus: 'completed',
+        nextStatus: 'active',
+      }),
+    ).toThrow('Invalid paired task status transition: completed -> active');
+  });
+
+  it('writes status transitions with optimistic revision guard', () => {
+    const updated = transitionPairedTaskStatus({
+      taskId: 'task-1',
+      currentStatus: 'active',
+      nextStatus: 'review_ready',
+      expectedUpdatedAt: '2026-04-29T01:00:00.000Z',
+      updatedAt: '2026-04-29T01:01:00.000Z',
+      patch: {
+        review_requested_at: '2026-04-29T01:01:00.000Z',
+      },
+    });
+
+    expect(updated).toBe(true);
+    expect(updatePairedTaskIfUnchanged).toHaveBeenCalledWith(
+      'task-1',
+      '2026-04-29T01:00:00.000Z',
+      {
+        review_requested_at: '2026-04-29T01:01:00.000Z',
+        status: 'review_ready',
+        updated_at: '2026-04-29T01:01:00.000Z',
+      },
+    );
+  });
+
+  it('logs stale status transitions without writing success', () => {
+    vi.mocked(updatePairedTaskIfUnchanged).mockReturnValue(false);
+
+    const updated = transitionPairedTaskStatus({
+      taskId: 'task-1',
+      currentStatus: 'active',
+      nextStatus: 'completed',
+      expectedUpdatedAt: 'old',
+      updatedAt: 'new',
+      patch: {
+        completion_reason: 'done',
+      },
+    });
+
+    expect(updated).toBe(false);
+    expect(logger.warn).toHaveBeenCalledWith(
+      {
+        taskId: 'task-1',
+        currentStatus: 'active',
+        nextStatus: 'completed',
+        expectedUpdatedAt: 'old',
+      },
+      'Skipped stale paired task status transition because the task revision changed',
+    );
+  });
+
+  it('applies non-status patches with optimistic revision guard', () => {
+    const updated = applyPairedTaskPatch({
+      taskId: 'task-1',
+      expectedUpdatedAt: 'old',
+      updatedAt: 'new',
+      patch: {
+        owner_failure_count: 2,
+      },
+    });
+
+    expect(updated).toBe(true);
+    expect(updatePairedTaskIfUnchanged).toHaveBeenCalledWith('task-1', 'old', {
+      owner_failure_count: 2,
+      updated_at: 'new',
+    });
+  });
+});

src/paired-task-status.ts

@@ -0,0 +1,139 @@
+import { updatePairedTaskIfUnchanged } from './db.js';
+import { logger } from './logger.js';
+import type { PairedTaskStatus } from './types.js';
+
+export const ALLOWED_PAIRED_STATUS_TRANSITIONS: Record<
+  PairedTaskStatus,
+  ReadonlySet<PairedTaskStatus>
+> = {
+  active: new Set(['review_ready', 'arbiter_requested', 'completed']),
+  review_ready: new Set([
+    'active',
+    'in_review',
+    'arbiter_requested',
+    'completed',
+  ]),
+  in_review: new Set([
+    'active',
+    'review_ready',
+    'merge_ready',
+    'arbiter_requested',
+    'completed',
+  ]),
+  merge_ready: new Set(['active', 'arbiter_requested', 'completed']),
+  completed: new Set(),
+  arbiter_requested: new Set(['in_arbitration', 'completed']),
+  in_arbitration: new Set(['active', 'arbiter_requested', 'completed']),
+};
+
+export function assertPairedTaskStatusTransition(args: {
+  currentStatus: PairedTaskStatus;
+  nextStatus: PairedTaskStatus;
+}): void {
+  const { currentStatus, nextStatus } = args;
+  if (currentStatus === nextStatus) {
+    return;
+  }
+
+  if (ALLOWED_PAIRED_STATUS_TRANSITIONS[currentStatus].has(nextStatus)) {
+    return;
+  }
+
+  throw new Error(
+    `Invalid paired task status transition: ${currentStatus} -> ${nextStatus}`,
+  );
+}
+
+export function transitionPairedTaskStatus(args: {
+  taskId: string;
+  currentStatus: PairedTaskStatus;
+  nextStatus: PairedTaskStatus;
+  expectedUpdatedAt: string;
+  updatedAt: string;
+  patch?: {
+    title?: string | null;
+    source_ref?: string | null;
+    plan_notes?: string | null;
+    review_requested_at?: string | null;
+    round_trip_count?: number;
+    owner_failure_count?: number;
+    owner_step_done_streak?: number;
+    finalize_step_done_count?: number;
+    task_done_then_user_reopen_count?: number;
+    empty_step_done_streak?: number;
+    arbiter_verdict?: string | null;
+    arbiter_requested_at?: string | null;
+    completion_reason?: string | null;
+  };
+}): boolean {
+  assertPairedTaskStatusTransition({
+    currentStatus: args.currentStatus,
+    nextStatus: args.nextStatus,
+  });
+
+  const updated = updatePairedTaskIfUnchanged(
+    args.taskId,
+    args.expectedUpdatedAt,
+    {
+      ...args.patch,
+      status: args.nextStatus,
+      updated_at: args.updatedAt,
+    },
+  );
+  if (!updated) {
+    logger.warn(
+      {
+        taskId: args.taskId,
+        currentStatus: args.currentStatus,
+        nextStatus: args.nextStatus,
+        expectedUpdatedAt: args.expectedUpdatedAt,
+      },
+      'Skipped stale paired task status transition because the task revision changed',
+    );
+  }
+  return updated;
+}
+
+export function applyPairedTaskPatch(args: {
+  taskId: string;
+  expectedUpdatedAt: string;
+  updatedAt: string;
+  patch: {
+    title?: string | null;
+    source_ref?: string | null;
+    plan_notes?: string | null;
+    review_requested_at?: string | null;
+    round_trip_count?: number;
+    owner_failure_count?: number;
+    owner_step_done_streak?: number;
+    finalize_step_done_count?: number;
+    task_done_then_user_reopen_count?: number;
+    empty_step_done_streak?: number;
+    status?: PairedTaskStatus;
+    arbiter_verdict?: string | null;
+    arbiter_requested_at?: string | null;
+    completion_reason?: string | null;
+  };
+}): boolean {
+  const updated = updatePairedTaskIfUnchanged(
+    args.taskId,
+    args.expectedUpdatedAt,
+    {
+      ...args.patch,
+      updated_at: args.updatedAt,
+    },
+  );
+  if (!updated) {
+    logger.warn(
+      {
+        taskId: args.taskId,
+        expectedUpdatedAt: args.expectedUpdatedAt,
+      },
+      'Skipped stale paired task patch because the task revision changed',
+    );
+  }
+  return updated;
+}
+
+export type TransitionPairedTaskStatusFn = typeof transitionPairedTaskStatus;
+export type ApplyPairedTaskPatchFn = typeof applyPairedTaskPatch;

src/paired-workspace-manager.ts

@@ -12,7 +12,7 @@ import {
 } from './db.js';
 import { resolvePairedTaskWorkspacePath } from './group-folder.js';
 import { logger } from './logger.js';
-import { transitionPairedTaskStatus } from './paired-execution-context-shared.js';
+import { transitionPairedTaskStatus } from './paired-task-status.js';
 import type { PairedTask, PairedWorkspace } from './types.js';
 import { ensureWorkspaceDependenciesInstalled } from './workspace-package-manager.js';