Move paired handoff enqueue into backend

2026-04-08 10:09:23 +09:00
parent 6efe0a77f9
commit d996d5ff10
5 changed files with 366 additions and 20 deletions
--- a/src/message-runtime-dispatch.ts
+++ b/src/message-runtime-dispatch.ts
@@ -67,7 +67,7 @@ export function enqueueGenericFollowUpAfterDeliveryRetry(args: {
        deliveryRole: args.deliveryRole,
        pendingTaskStatus: args.pendingTask?.status ?? null,
      },
-      'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff',
+      'No queued follow-up was required after delivery retry',
    );
    return;
  }
@@ -90,6 +90,21 @@ export function enqueueGenericFollowUpAfterDeliveryRetry(args: {
    nextTurnAction.kind,
    args.runId,
  );
+  if (scheduled) {
+    args.log.info(
+      {
+        workItemId: args.workItemId,
+        chatJid: args.chatJid,
+        deliveryRole: args.deliveryRole,
+        taskId: args.pendingTask?.id ?? null,
+        pendingTaskStatus: args.pendingTask?.status ?? null,
+        taskStatus: args.pendingTask?.status ?? null,
+        intentKind: nextTurnAction.kind,
+      },
+      'Queued paired follow-up after delivery retry',
+    );
+    return;
+  }
  if (!scheduled) {
    args.log.info(
      {
@@ -97,6 +112,7 @@ export function enqueueGenericFollowUpAfterDeliveryRetry(args: {
        chatJid: args.chatJid,
        deliveryRole: args.deliveryRole,
        taskId: args.pendingTask?.id ?? null,
+        pendingTaskStatus: args.pendingTask?.status ?? null,
        taskStatus: args.pendingTask?.status ?? null,
        intentKind: nextTurnAction.kind,
      },
--- a/src/message-runtime-rules.test.ts
+++ b/src/message-runtime-rules.test.ts
@@ -105,6 +105,39 @@ describe('message-runtime-rules', () => {
    ).toEqual({ kind: 'none' });
  });

+  it('dispatches reviewer and arbiter delivery success through paired follow-up enqueue when a handoff is pending', () => {
+    expect(
+      resolveFollowUpDispatch({
+        source: 'delivery-success',
+        nextTurnAction: { kind: 'owner-follow-up' },
+        completedRole: 'reviewer',
+      }),
+    ).toEqual({
+      kind: 'enqueue',
+      queueKind: 'paired-follow-up',
+    });
+    expect(
+      resolveFollowUpDispatch({
+        source: 'delivery-success',
+        nextTurnAction: { kind: 'finalize-owner-turn' },
+        completedRole: 'reviewer',
+      }),
+    ).toEqual({
+      kind: 'enqueue',
+      queueKind: 'paired-follow-up',
+    });
+    expect(
+      resolveFollowUpDispatch({
+        source: 'delivery-success',
+        nextTurnAction: { kind: 'owner-follow-up' },
+        completedRole: 'arbiter',
+      }),
+    ).toEqual({
+      kind: 'enqueue',
+      queueKind: 'paired-follow-up',
+    });
+  });
+
  it('dispatches delivery retry follow-ups through either generic message checks or paired follow-up enqueue', () => {
    expect(
      resolveFollowUpDispatch({
@@ -132,7 +165,10 @@ describe('message-runtime-rules', () => {
        nextTurnAction: { kind: 'finalize-owner-turn' },
        completedRole: 'reviewer',
      }),
-    ).toEqual({ kind: 'none' });
+    ).toEqual({
+      kind: 'enqueue',
+      queueKind: 'paired-follow-up',
+    });
  });

  it('dispatches bot-only follow-ups through inline finalize or paired enqueue', () => {
--- a/src/message-runtime-rules.ts
+++ b/src/message-runtime-rules.ts
@@ -118,6 +118,7 @@ export function resolveNextTurnAction(args: {

 export function resolveFollowUpDispatch(args: {
  source:
+    | 'delivery-success'
    | 'owner-delivery-success'
    | 'delivery-retry'
    | 'bot-only-follow-up'
@@ -128,18 +129,21 @@ export function resolveFollowUpDispatch(args: {
  sawOutput?: boolean;
 }): FollowUpDispatch {
  switch (args.source) {
+    case 'delivery-success':
    case 'owner-delivery-success':
-      return args.nextTurnAction.kind === 'reviewer-turn'
-        ? { kind: 'enqueue', queueKind: 'paired-follow-up' }
-        : { kind: 'none' };
+      if (
+        args.source === 'owner-delivery-success' ||
+        args.completedRole === 'owner'
+      ) {
+        return args.nextTurnAction.kind === 'reviewer-turn'
+          ? { kind: 'enqueue', queueKind: 'paired-follow-up' }
+          : { kind: 'none' };
+      }
+      return args.nextTurnAction.kind === 'none'
+        ? { kind: 'none' }
+        : { kind: 'enqueue', queueKind: 'paired-follow-up' };

    case 'delivery-retry':
-      if (
-        args.completedRole === 'reviewer' &&
-        args.nextTurnAction.kind === 'finalize-owner-turn'
-      ) {
-        return { kind: 'none' };
-      }
      if (args.nextTurnAction.kind === 'none') {
        return { kind: 'enqueue', queueKind: 'message-check' };
      }
--- a/src/message-runtime.test.ts
+++ b/src/message-runtime.test.ts
@@ -882,15 +882,17 @@ describe('createMessageRuntime', () => {
      'reviewer final retry',
    );
    expect(ownerChannel.sendMessage).not.toHaveBeenCalled();
-    expect(enqueueMessageCheck).not.toHaveBeenCalled();
+    expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid);
    expect(logger.info).toHaveBeenCalledWith(
      expect.objectContaining({
        workItemId: 101,
        chatJid,
        deliveryRole: 'reviewer',
+        taskId: 'task-review-delivery-role',
        pendingTaskStatus: 'merge_ready',
+        intentKind: 'finalize-owner-turn',
      }),
-      'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff',
+      'Queued paired follow-up after delivery retry',
    );
  });

@@ -975,15 +977,17 @@ describe('createMessageRuntime', () => {
      'fallback reviewer final retry',
    );
    expect(ownerChannel.sendMessage).not.toHaveBeenCalled();
-    expect(enqueueMessageCheck).not.toHaveBeenCalled();
+    expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid);
    expect(logger.info).toHaveBeenCalledWith(
      expect.objectContaining({
        workItemId: 102,
        chatJid,
        deliveryRole: 'reviewer',
+        taskId: 'task-fallback-review-delivery-role',
        pendingTaskStatus: 'merge_ready',
+        intentKind: 'finalize-owner-turn',
      }),
-      'Skipping queued follow-up after reviewer merge_ready delivery because inline finalize will handle the handoff',
+      'Queued paired follow-up after delivery retry',
    );
  });

@@ -1920,6 +1924,287 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
    );
  });

+  it('re-enqueues owner after a successful reviewer delivery moves the task back to active', async () => {
+    const chatJid = 'group@test';
+    const group = makeGroup('codex');
+    const ownerChannel = makeChannel(chatJid);
+    const reviewerChannel = makeChannel(chatJid, 'discord-review', false);
+    const enqueueMessageCheck = vi.fn();
+    const pairedTask = {
+      id: 'task-reviewer-delivery-owner-follow-up',
+      chat_jid: chatJid,
+      group_folder: group.folder,
+      owner_service_id: 'claude',
+      reviewer_service_id: 'codex-main',
+      title: null,
+      source_ref: 'HEAD',
+      plan_notes: null,
+      review_requested_at: '2026-03-30T00:00:00.000Z',
+      round_trip_count: 1,
+      status: 'review_ready',
+      arbiter_verdict: null,
+      arbiter_requested_at: null,
+      completion_reason: null,
+      created_at: '2026-03-30T00:00:00.000Z',
+      updated_at: '2026-03-30T00:00:00.000Z',
+    } as any;
+
+    vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true);
+    vi.mocked(db.getLatestOpenPairedTaskForChat).mockImplementation(
+      () => pairedTask,
+    );
+    vi.mocked(agentRunner.runAgentProcess).mockImplementation(
+      async (_group, _input, _onProcess, onOutput) => {
+        pairedTask.status = 'active';
+        await onOutput?.({
+          status: 'success',
+          phase: 'final',
+          result: 'DONE_WITH_CONCERNS\nreviewer follow-up needed',
+          newSessionId: 'session-reviewer-delivery-owner-follow-up',
+        });
+        return {
+          status: 'success',
+          result: 'DONE_WITH_CONCERNS\nreviewer follow-up needed',
+          newSessionId: 'session-reviewer-delivery-owner-follow-up',
+        };
+      },
+    );
+
+    const runtime = createMessageRuntime({
+      assistantName: 'Andy',
+      idleTimeout: 1_000,
+      pollInterval: 1_000,
+      timezone: 'UTC',
+      triggerPattern: /^@Andy\b/i,
+      channels: [ownerChannel, reviewerChannel],
+      queue: {
+        registerProcess: vi.fn(),
+        closeStdin: vi.fn(),
+        notifyIdle: vi.fn(),
+        enqueueMessageCheck,
+      } as any,
+      getRegisteredGroups: () => ({ [chatJid]: group }),
+      getSessions: () => ({}),
+      getLastTimestamp: () => '',
+      setLastTimestamp: vi.fn(),
+      getLastAgentTimestamps: () => ({}),
+      saveState: vi.fn(),
+      persistSession: vi.fn(),
+      clearSession: vi.fn(),
+    });
+
+    const result = await runtime.processGroupMessages(chatJid, {
+      runId: 'run-reviewer-delivery-owner-follow-up',
+      reason: 'messages',
+    });
+
+    expect(result).toBe(true);
+    expect(reviewerChannel.sendMessage).toHaveBeenCalledWith(
+      chatJid,
+      'DONE_WITH_CONCERNS\nreviewer follow-up needed',
+    );
+    expect(ownerChannel.sendMessage).not.toHaveBeenCalled();
+    expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid);
+    expect(logger.info).toHaveBeenCalledWith(
+      expect.objectContaining({
+        chatJid,
+        runId: 'run-reviewer-delivery-owner-follow-up',
+        completedRole: 'reviewer',
+        taskId: 'task-reviewer-delivery-owner-follow-up',
+        taskStatus: 'active',
+      }),
+      'Queued paired follow-up after successful reviewer/arbiter delivery',
+    );
+  });
+
+  it('re-enqueues finalize-owner after a successful reviewer delivery moves the task to merge_ready', async () => {
+    const chatJid = 'group@test';
+    const group = makeGroup('codex');
+    const ownerChannel = makeChannel(chatJid);
+    const reviewerChannel = makeChannel(chatJid, 'discord-review', false);
+    const enqueueMessageCheck = vi.fn();
+    const pairedTask = {
+      id: 'task-reviewer-delivery-finalize-owner',
+      chat_jid: chatJid,
+      group_folder: group.folder,
+      owner_service_id: 'claude',
+      reviewer_service_id: 'codex-main',
+      title: null,
+      source_ref: 'HEAD',
+      plan_notes: null,
+      review_requested_at: '2026-03-30T00:00:00.000Z',
+      round_trip_count: 1,
+      status: 'review_ready',
+      arbiter_verdict: null,
+      arbiter_requested_at: null,
+      completion_reason: null,
+      created_at: '2026-03-30T00:00:00.000Z',
+      updated_at: '2026-03-30T00:00:00.000Z',
+    } as any;
+
+    vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true);
+    vi.mocked(db.getLatestOpenPairedTaskForChat).mockImplementation(
+      () => pairedTask,
+    );
+    vi.mocked(agentRunner.runAgentProcess).mockImplementation(
+      async (_group, _input, _onProcess, onOutput) => {
+        pairedTask.status = 'merge_ready';
+        await onOutput?.({
+          status: 'success',
+          phase: 'final',
+          result: 'DONE\nreview approved',
+          newSessionId: 'session-reviewer-delivery-finalize-owner',
+        });
+        return {
+          status: 'success',
+          result: 'DONE\nreview approved',
+          newSessionId: 'session-reviewer-delivery-finalize-owner',
+        };
+      },
+    );
+
+    const runtime = createMessageRuntime({
+      assistantName: 'Andy',
+      idleTimeout: 1_000,
+      pollInterval: 1_000,
+      timezone: 'UTC',
+      triggerPattern: /^@Andy\b/i,
+      channels: [ownerChannel, reviewerChannel],
+      queue: {
+        registerProcess: vi.fn(),
+        closeStdin: vi.fn(),
+        notifyIdle: vi.fn(),
+        enqueueMessageCheck,
+      } as any,
+      getRegisteredGroups: () => ({ [chatJid]: group }),
+      getSessions: () => ({}),
+      getLastTimestamp: () => '',
+      setLastTimestamp: vi.fn(),
+      getLastAgentTimestamps: () => ({}),
+      saveState: vi.fn(),
+      persistSession: vi.fn(),
+      clearSession: vi.fn(),
+    });
+
+    const result = await runtime.processGroupMessages(chatJid, {
+      runId: 'run-reviewer-delivery-finalize-owner',
+      reason: 'messages',
+    });
+
+    expect(result).toBe(true);
+    expect(reviewerChannel.sendMessage).toHaveBeenCalledWith(
+      chatJid,
+      'DONE\nreview approved',
+    );
+    expect(ownerChannel.sendMessage).not.toHaveBeenCalled();
+    expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid);
+    expect(logger.info).toHaveBeenCalledWith(
+      expect.objectContaining({
+        chatJid,
+        runId: 'run-reviewer-delivery-finalize-owner',
+        completedRole: 'reviewer',
+        taskId: 'task-reviewer-delivery-finalize-owner',
+        taskStatus: 'merge_ready',
+      }),
+      'Queued paired follow-up after successful reviewer/arbiter delivery',
+    );
+  });
+
+  it('re-enqueues owner after a successful arbiter delivery moves the task back to active', async () => {
+    const chatJid = 'group@test';
+    const group = makeGroup('codex');
+    const ownerChannel = makeChannel(chatJid);
+    const reviewerChannel = makeChannel(chatJid, 'discord-review', false);
+    const arbiterChannel = makeChannel(chatJid, 'discord-arbiter', false);
+    const enqueueMessageCheck = vi.fn();
+    const pairedTask = {
+      id: 'task-arbiter-delivery-owner-follow-up',
+      chat_jid: chatJid,
+      group_folder: group.folder,
+      owner_service_id: 'claude',
+      reviewer_service_id: 'codex-main',
+      arbiter_service_id: 'claude-arbiter',
+      title: null,
+      source_ref: 'HEAD',
+      plan_notes: null,
+      review_requested_at: '2026-03-30T00:00:00.000Z',
+      round_trip_count: 1,
+      status: 'arbiter_requested',
+      arbiter_verdict: null,
+      arbiter_requested_at: '2026-03-30T00:00:10.000Z',
+      completion_reason: null,
+      created_at: '2026-03-30T00:00:00.000Z',
+      updated_at: '2026-03-30T00:00:00.000Z',
+    } as any;
+
+    vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true);
+    vi.mocked(db.getLatestOpenPairedTaskForChat).mockImplementation(
+      () => pairedTask,
+    );
+    vi.mocked(agentRunner.runAgentProcess).mockImplementation(
+      async (_group, _input, _onProcess, onOutput) => {
+        pairedTask.status = 'active';
+        await onOutput?.({
+          status: 'success',
+          phase: 'final',
+          result: 'DONE_WITH_CONCERNS\narbiter says revise',
+          newSessionId: 'session-arbiter-delivery-owner-follow-up',
+        });
+        return {
+          status: 'success',
+          result: 'DONE_WITH_CONCERNS\narbiter says revise',
+          newSessionId: 'session-arbiter-delivery-owner-follow-up',
+        };
+      },
+    );
+
+    const runtime = createMessageRuntime({
+      assistantName: 'Andy',
+      idleTimeout: 1_000,
+      pollInterval: 1_000,
+      timezone: 'UTC',
+      triggerPattern: /^@Andy\b/i,
+      channels: [ownerChannel, reviewerChannel, arbiterChannel],
+      queue: {
+        registerProcess: vi.fn(),
+        closeStdin: vi.fn(),
+        notifyIdle: vi.fn(),
+        enqueueMessageCheck,
+      } as any,
+      getRegisteredGroups: () => ({ [chatJid]: group }),
+      getSessions: () => ({}),
+      getLastTimestamp: () => '',
+      setLastTimestamp: vi.fn(),
+      getLastAgentTimestamps: () => ({}),
+      saveState: vi.fn(),
+      persistSession: vi.fn(),
+      clearSession: vi.fn(),
+    });
+
+    const result = await runtime.processGroupMessages(chatJid, {
+      runId: 'run-arbiter-delivery-owner-follow-up',
+      reason: 'messages',
+    });
+
+    expect(result).toBe(true);
+    expect(arbiterChannel.sendMessage).toHaveBeenCalledWith(
+      chatJid,
+      'DONE_WITH_CONCERNS\narbiter says revise',
+    );
+    expect(ownerChannel.sendMessage).not.toHaveBeenCalled();
+    expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid);
+    expect(logger.info).toHaveBeenCalledWith(
+      expect.objectContaining({
+        chatJid,
+        runId: 'run-arbiter-delivery-owner-follow-up',
+        completedRole: 'arbiter',
+        taskId: 'task-arbiter-delivery-owner-follow-up',
+        taskStatus: 'active',
+      }),
+      'Queued paired follow-up after successful reviewer/arbiter delivery',
+    );
+  });
+
  it('does not enqueue the same reviewer follow-up twice across different runs while task state is unchanged', async () => {
    const chatJid = 'group@test';
    const group = makeGroup('codex');
--- a/src/message-runtime.ts
+++ b/src/message-runtime.ts
@@ -343,16 +343,17 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
      const { deliverySucceeded, visiblePhase } =
        await turnController.finish(outputStatus);

-      if (deliverySucceeded && pairedRoom && resolvedDeliveryRole === 'owner') {
+      if (deliverySucceeded && pairedRoom && resolvedDeliveryRole) {
        const pendingTaskAfterDelivery =
          getLatestOpenPairedTaskForChat(chatJid);
        const nextTurnAction = resolveNextTurnAction({
          taskStatus: pendingTaskAfterDelivery?.status,
-          lastTurnOutputRole: 'owner',
+          lastTurnOutputRole: resolvedDeliveryRole,
        });
        const dispatch = resolveFollowUpDispatch({
-          source: 'owner-delivery-success',
+          source: 'delivery-success',
          nextTurnAction,
+          completedRole: resolvedDeliveryRole,
        });
        if (
          dispatch.kind === 'enqueue' &&
@@ -376,8 +377,12 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
              scheduled,
            },
            scheduled
-              ? 'Queued paired follow-up after successful owner delivery'
-              : 'Skipped duplicate paired follow-up after successful owner delivery while task state was unchanged',
+              ? resolvedDeliveryRole === 'owner'
+                ? 'Queued paired follow-up after successful owner delivery'
+                : 'Queued paired follow-up after successful reviewer/arbiter delivery'
+              : resolvedDeliveryRole === 'owner'
+                ? 'Skipped duplicate paired follow-up after successful owner delivery while task state was unchanged'
+                : 'Skipped duplicate paired follow-up after successful reviewer/arbiter delivery while task state was unchanged',
          );
        }
      }