Merge pull request #666 from mhenke/fix/661-idle-reconciliation-race

fix(scheduler): defer idle reconciliation to avoid dropping late injected completions

src/hooks/task-session-manager/index.test.ts

@@ -10,6 +10,11 @@ import {
   createTaskSessionManagerHook,
 } from './index';
 
+/** Wait for the idle reconciliation delay (2s + margin) to flush. */
+function flushIdleReconcileDelay() {
+  return new Promise((resolve) => setTimeout(resolve, 2100));
+}
+
 function createHook(options?: {
   shouldManageSession?: (sessionID: string) => boolean;
   readContextMinLines?: number;
@@ -54,6 +59,21 @@ function createMessages(sessionID: string, text = 'user message') {
   };
 }
 
+function setupCompletedJob(
+  board: BackgroundJobBoard,
+  opts?: { taskID?: string; parentSessionID?: string },
+) {
+  const taskID = opts?.taskID ?? 'child-1';
+  const parentSessionID = opts?.parentSessionID ?? 'parent-1';
+  board.registerLaunch({
+    taskID,
+    parentSessionID,
+    agent: 'oracle',
+    description: 'review plan',
+  });
+  board.updateStatus({ taskID, state: 'completed', resultSummary: 'done' });
+}
+
 describe('task-session-manager hook', () => {
   test('ignores messages without OpenCode info or parts', async () => {
     const board = new BackgroundJobBoard();
@@ -1281,6 +1301,9 @@ describe('task-session-manager hook', () => {
       },
     });
 
+    // Wait for deferred idle reconciliation timeout
+    await flushIdleReconcileDelay();
+
     expect(board.get('child-1')).toMatchObject({
       state: 'reconciled',
       terminalUnreconciled: false,
@@ -1320,6 +1343,40 @@ describe('task-session-manager hook', () => {
     });
   });
 
+  test('late injected completion during idle delay is not dropped by reconciliation', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    setupCompletedJob(board);
+
+    const messages = createMessages('parent-1', 'continue');
+    await hook['experimental.chat.messages.transform']({}, messages);
+
+    // Fire idle event (starts 2s reconciliation timer)
+    await hook.event({
+      event: {
+        type: 'session.status',
+        properties: { sessionID: 'parent-1', status: { type: 'idle' } },
+      },
+    });
+
+    // Before the timer fires, a late injected completion arrives with error
+    board.updateStatus({
+      taskID: 'child-1',
+      state: 'error',
+      resultSummary: 'actual error from child',
+    });
+
+    await flushIdleReconcileDelay();
+
+    // Reconciled with the late error's result, not the idle-written fallback
+    expect(board.get('child-1')).toMatchObject({
+      state: 'reconciled',
+      terminalState: 'error',
+      resultSummary: 'actual error from child',
+    });
+  });
+
   test('does not reconcile terminal jobs before they are injected into a prompt', async () => {
     const board = new BackgroundJobBoard();
     const { hook } = createHook({ backgroundJobBoard: board });
@@ -1407,6 +1464,9 @@ describe('task-session-manager hook', () => {
       },
     });
 
+    // Wait for deferred idle reconciliation timeout
+    await flushIdleReconcileDelay();
+
     const nextMessages = createMessages('parent-1', 'reuse');
     await hook['experimental.chat.messages.transform']({}, nextMessages);
     expect(nextMessages.messages[0].parts[0].text).toContain(
@@ -1684,6 +1744,9 @@ describe('task-session-manager hook', () => {
       },
     });
 
+    // Wait for deferred idle reconciliation timeout
+    await flushIdleReconcileDelay();
+
     const reusable = createMessages('parent-1', 'reuse');
     await hook['experimental.chat.messages.transform']({}, reusable);
     expect(reusable.messages[0].parts[0].text).toContain(
@@ -1814,6 +1877,10 @@ describe('task-session-manager hook', () => {
         properties: { sessionID: 'parent-1', status: { type: 'idle' } },
       },
     });
+
+    // Wait for deferred idle reconciliation timeout
+    await flushIdleReconcileDelay();
+
     const next = createMessages('parent-1', 'reuse');
     await hook['experimental.chat.messages.transform']({}, next);
     const prompt = next.messages[0].parts[0].text;

src/hooks/task-session-manager/index.ts

@@ -39,6 +39,18 @@ const BACKGROUND_COMPLETION_FAILED = /^Background task failed: /;
 const MAX_PROCESSED_INJECTED_COMPLETIONS = 500;
 const RAW_SESSION_ID_PATTERN = /^ses_[A-Za-z0-9_-]+$/;
 
+/**
+ * Delay before reconciling idle sessions.
+ * Gives late injected completions time to arrive within this window.
+ * Completions arriving after the window are still dropped (the race is reduced, not eliminated).
+ * ponytail: fixed timeout — event-driven confirmation would fully close the race but adds
+ * significant complexity for a case that rarely exceeds this window in practice.
+ */
+const IDLE_RECONCILE_DELAY_MS = 2_000;
+
+/** Track idle reconciliation timers to cancel on busy/error/deleted. */
+const idleReconcileTimers = new Map<string, ReturnType<typeof setTimeout>>();
+
 function djb2Hash(str: string): string {
   let hash = 5381;
   for (let i = 0; i < str.length; i++) {
@@ -622,8 +634,11 @@ export function createTaskSessionManagerHook(
           runningJobForSession: job?.state === 'running' || false,
         });
         if (sessionId && options.shouldManageSession(sessionId)) {
-          reconcileInjectedTerminalJobs(sessionId);
-          return;
+          const timer = setTimeout(() => {
+            idleReconcileTimers.delete(sessionId);
+            reconcileInjectedTerminalJobs(sessionId);
+          }, IDLE_RECONCILE_DELAY_MS).unref?.();
+          idleReconcileTimers.set(sessionId, timer);
         }
 
         // Fallback: for background child sessions that go idle without
@@ -663,6 +678,13 @@ export function createTaskSessionManagerHook(
       if (input.event.type === 'session.error') {
         const sessionId =
           input.event.properties?.info?.id || input.event.properties?.sessionID;
+        if (sessionId) {
+          const timer = idleReconcileTimers.get(sessionId);
+          if (timer) {
+            clearTimeout(timer);
+            idleReconcileTimers.delete(sessionId);
+          }
+        }
         if (sessionId && options.shouldManageSession(sessionId)) {
           // Only clear injected terminal jobs for fatal errors.
           // Rate-limit errors are recovered by ForegroundFallbackManager
@@ -687,6 +709,13 @@ export function createTaskSessionManagerHook(
       ) {
         const sessionId =
           input.event.properties?.info?.id || input.event.properties?.sessionID;
+        if (sessionId) {
+          const timer = idleReconcileTimers.get(sessionId);
+          if (timer) {
+            clearTimeout(timer);
+            idleReconcileTimers.delete(sessionId);
+          }
+        }
         const before = sessionId
           ? backgroundJobBoard.get(sessionId)
           : undefined;
@@ -723,6 +752,12 @@ export function createTaskSessionManagerHook(
         input.event.properties?.info?.id || input.event.properties?.sessionID;
       if (!sessionId) return;
 
+      const timer = idleReconcileTimers.get(sessionId);
+      if (timer) {
+        clearTimeout(timer);
+        idleReconcileTimers.delete(sessionId);
+      }
+
       log('[task-session-manager] session.deleted observed', {
         sessionID: sessionId,
       });