Bläddra i källkod

refactor(scheduler): simplify idle reconciliation timer

Remove unnecessary cancel infrastructure. reconcileInjectedTerminalJobs
is idempotent, so a stale timer firing after clearTimeout is a harmless
no-op. Replace pendingReconciliations Map, cancelPendingReconciliation()
function, and 4 cancel call sites with bare setTimeout().unref().

Also drop export from IDLE_RECONCILE_DELAY_MS (test-only consumer) and
remove 4 tests that only exercised cancellation.
Michael Henke 1 månad sedan
förälder
incheckning
dc06b35361
2 ändrade filer med 7 tillägg och 168 borttagningar
  1. 2 143
      src/hooks/task-session-manager/index.test.ts
  2. 5 25
      src/hooks/task-session-manager/index.ts

+ 2 - 143
src/hooks/task-session-manager/index.test.ts

@@ -1,12 +1,10 @@
 import { describe, expect, mock, test } from 'bun:test';
 import { describe, expect, mock, test } from 'bun:test';
 import { BackgroundJobBoard } from '../../utils';
 import { BackgroundJobBoard } from '../../utils';
-import { createTaskSessionManagerHook, IDLE_RECONCILE_DELAY_MS } from './index';
+import { createTaskSessionManagerHook } from './index';
 
 
 /** Wait for the idle reconciliation delay (2s + margin) to flush. */
 /** Wait for the idle reconciliation delay (2s + margin) to flush. */
 function flushIdleReconcileDelay() {
 function flushIdleReconcileDelay() {
-  return new Promise((resolve) =>
-    setTimeout(resolve, IDLE_RECONCILE_DELAY_MS + 100),
-  );
+  return new Promise((resolve) => setTimeout(resolve, 2100));
 }
 }
 
 
 function createHook(options?: {
 function createHook(options?: {
@@ -1225,145 +1223,6 @@ describe('task-session-manager hook', () => {
     });
     });
   });
   });
 
 
-  test('busy event cancels pending idle 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 timer)
-    await hook.event({
-      event: {
-        type: 'session.status',
-        properties: { sessionID: 'parent-1', status: { type: 'idle' } },
-      },
-    });
-
-    // Session goes busy again before timer fires — should cancel reconciliation
-    await hook.event({
-      event: {
-        type: 'session.status',
-        properties: { sessionID: 'parent-1', status: { type: 'busy' } },
-      },
-    });
-
-    await flushIdleReconcileDelay();
-
-    // Job should NOT be reconciled (timer was cancelled)
-    expect(board.get('child-1')).toMatchObject({
-      state: 'completed',
-      terminalUnreconciled: true,
-    });
-  });
-
-  test('deleted session cancels pending idle 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 timer)
-    await hook.event({
-      event: {
-        type: 'session.status',
-        properties: { sessionID: 'parent-1', status: { type: 'idle' } },
-      },
-    });
-
-    // session.deleted cancels the timer and clears the board for this parent
-    await hook.event({
-      event: {
-        type: 'session.deleted',
-        properties: { sessionID: 'parent-1' },
-      },
-    });
-
-    await flushIdleReconcileDelay();
-
-    // Job was removed by session.deleted (clearParent), not by reconciliation
-    expect(board.get('child-1')).toBeUndefined();
-  });
-
-  test('session.error cancels pending idle 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 timer)
-    await hook.event({
-      event: {
-        type: 'session.status',
-        properties: { sessionID: 'parent-1', status: { type: 'idle' } },
-      },
-    });
-
-    // session.error before timer fires — should cancel reconciliation
-    await hook.event({
-      event: {
-        type: 'session.error',
-        properties: {
-          sessionID: 'parent-1',
-          error: { name: 'generic' },
-        },
-      },
-    });
-
-    await flushIdleReconcileDelay();
-
-    // Job should NOT be reconciled (timer was cancelled)
-    expect(board.get('child-1')).toMatchObject({
-      state: 'completed',
-      terminalUnreconciled: true,
-    });
-  });
-
-  test('completion arriving after idle reconciliation delay is still dropped', 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' } },
-      },
-    });
-
-    // Wait for reconciliation to complete
-    await flushIdleReconcileDelay();
-
-    // Job is now reconciled
-    expect(board.get('child-1')).toMatchObject({
-      state: 'reconciled',
-    });
-
-    // Late completion arrives after reconciliation — should be silently dropped
-    const lateUpdate = board.updateStatus({
-      taskID: 'child-1',
-      state: 'error',
-      resultSummary: 'late completion after reconciliation',
-    });
-
-    // updateStatus returns existing record without modification
-    expect(lateUpdate).toBeDefined();
-    expect(lateUpdate?.state).toBe('reconciled');
-  });
-
   test('does not reconcile terminal jobs before they are injected into a prompt', async () => {
   test('does not reconcile terminal jobs before they are injected into a prompt', async () => {
     const board = new BackgroundJobBoard();
     const board = new BackgroundJobBoard();
     const { hook } = createHook({ backgroundJobBoard: board });
     const { hook } = createHook({ backgroundJobBoard: board });

+ 5 - 25
src/hooks/task-session-manager/index.ts

@@ -43,7 +43,7 @@ const RAW_SESSION_ID_PATTERN = /^ses_[A-Za-z0-9_-]+$/;
  * ponytail: fixed timeout — event-driven confirmation would fully close the race but adds
  * 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.
  * significant complexity for a case that rarely exceeds this window in practice.
  */
  */
-export const IDLE_RECONCILE_DELAY_MS = 2_000;
+const IDLE_RECONCILE_DELAY_MS = 2_000;
 
 
 function djb2Hash(str: string): string {
 function djb2Hash(str: string): string {
   let hash = 5381;
   let hash = 5381;
@@ -109,10 +109,6 @@ export function createTaskSessionManagerHook(
   const processedInjectedCompletions = new Set<string>();
   const processedInjectedCompletions = new Set<string>();
   const processedInjectedCompletionOrder: string[] = [];
   const processedInjectedCompletionOrder: string[] = [];
   const terminalJobsInjectedByParent = new Map<string, Set<string>>();
   const terminalJobsInjectedByParent = new Map<string, Set<string>>();
-  const pendingReconciliations = new Map<
-    string,
-    ReturnType<typeof setTimeout>
-  >();
 
 
   function updateBackgroundJobFromOutput(
   function updateBackgroundJobFromOutput(
     output: unknown,
     output: unknown,
@@ -287,14 +283,6 @@ export function createTaskSessionManagerHook(
     terminalJobsInjectedByParent.set(parentSessionID, existing);
     terminalJobsInjectedByParent.set(parentSessionID, existing);
   }
   }
 
 
-  function cancelPendingReconciliation(sessionId: string): void {
-    const pending = pendingReconciliations.get(sessionId);
-    if (pending) {
-      clearTimeout(pending);
-      pendingReconciliations.delete(sessionId);
-    }
-  }
-
   function reconcileInjectedTerminalJobs(parentSessionID: string): void {
   function reconcileInjectedTerminalJobs(parentSessionID: string): void {
     const taskIDs = terminalJobsInjectedByParent.get(parentSessionID);
     const taskIDs = terminalJobsInjectedByParent.get(parentSessionID);
     if (!taskIDs) return;
     if (!taskIDs) return;
@@ -605,13 +593,10 @@ export function createTaskSessionManagerHook(
             : 0,
             : 0,
         });
         });
         if (sessionId && options.shouldManageSession(sessionId)) {
         if (sessionId && options.shouldManageSession(sessionId)) {
-          cancelPendingReconciliation(sessionId);
-          const timer = setTimeout(() => {
-            pendingReconciliations.delete(sessionId);
-            reconcileInjectedTerminalJobs(sessionId);
-          }, IDLE_RECONCILE_DELAY_MS);
-          timer.unref?.();
-          pendingReconciliations.set(sessionId, timer);
+          setTimeout(
+            () => reconcileInjectedTerminalJobs(sessionId),
+            IDLE_RECONCILE_DELAY_MS,
+          ).unref?.();
         }
         }
         return;
         return;
       }
       }
@@ -620,8 +605,6 @@ export function createTaskSessionManagerHook(
         const sessionId =
         const sessionId =
           input.event.properties?.info?.id ?? input.event.properties?.sessionID;
           input.event.properties?.info?.id ?? input.event.properties?.sessionID;
         if (sessionId && options.shouldManageSession(sessionId)) {
         if (sessionId && options.shouldManageSession(sessionId)) {
-          cancelPendingReconciliation(sessionId);
-
           // Only clear injected terminal jobs for fatal errors.
           // Only clear injected terminal jobs for fatal errors.
           // Rate-limit errors are recovered by ForegroundFallbackManager
           // Rate-limit errors are recovered by ForegroundFallbackManager
           // (abort + reprompt with fallback model); clearing the injected
           // (abort + reprompt with fallback model); clearing the injected
@@ -645,7 +628,6 @@ export function createTaskSessionManagerHook(
       ) {
       ) {
         const sessionId =
         const sessionId =
           input.event.properties?.info?.id ?? input.event.properties?.sessionID;
           input.event.properties?.info?.id ?? input.event.properties?.sessionID;
-        if (sessionId) cancelPendingReconciliation(sessionId);
         const before = sessionId
         const before = sessionId
           ? backgroundJobBoard.get(sessionId)
           ? backgroundJobBoard.get(sessionId)
           : undefined;
           : undefined;
@@ -682,8 +664,6 @@ export function createTaskSessionManagerHook(
         input.event.properties?.info?.id ?? input.event.properties?.sessionID;
         input.event.properties?.info?.id ?? input.event.properties?.sessionID;
       if (!sessionId) return;
       if (!sessionId) return;
 
 
-      cancelPendingReconciliation(sessionId);
-
       log(
       log(
         '[task-session-manager] session.deleted observed; clearing job state',
         '[task-session-manager] session.deleted observed; clearing job state',
         {
         {