Эх сурвалжийг харах

fix(scheduler): address review feedback — guard idle reconciliation, add bookkeeping, add coverage

Michael Henke 1 сар өмнө
parent
commit
57760ea649

+ 6 - 0
src/hooks/foreground-fallback/index.ts

@@ -100,6 +100,12 @@ export class ForegroundFallbackManager {
    *  new fallback model also fails within the dedup window. */
    *  new fallback model also fails within the dedup window. */
   private readonly lastTriggerModel = new Map<string, string>();
   private readonly lastTriggerModel = new Map<string, string>();
 
 
+  /** Exposed for task-session-manager: prevents idle reconciliation
+   *  while a fallback abort/re-prompt is in flight for this session. */
+  isFallbackInProgress(sessionID: string): boolean {
+    return this.inProgress.has(sessionID);
+  }
+
   constructor(
   constructor(
     private readonly client: OpencodeClient,
     private readonly client: OpencodeClient,
     /**
     /**

+ 155 - 0
src/hooks/task-session-manager/index.test.ts

@@ -8,6 +8,7 @@ function createHook(options?: {
   readContextMaxFiles?: number;
   readContextMaxFiles?: number;
   backgroundJobBoard?: BackgroundJobBoard;
   backgroundJobBoard?: BackgroundJobBoard;
   sessionStatus?: unknown;
   sessionStatus?: unknown;
+  isFallbackInProgress?: (sessionID: string) => boolean;
 }) {
 }) {
   const hook = createTaskSessionManagerHook(
   const hook = createTaskSessionManagerHook(
     {
     {
@@ -25,6 +26,7 @@ function createHook(options?: {
       readContextMaxFiles: options?.readContextMaxFiles,
       readContextMaxFiles: options?.readContextMaxFiles,
       backgroundJobBoard: options?.backgroundJobBoard,
       backgroundJobBoard: options?.backgroundJobBoard,
       shouldManageSession: options?.shouldManageSession ?? (() => true),
       shouldManageSession: options?.shouldManageSession ?? (() => true),
+      isFallbackInProgress: options?.isFallbackInProgress,
     },
     },
   );
   );
 
 
@@ -1872,6 +1874,159 @@ describe('task-session-manager hook', () => {
     });
     });
   });
   });
 
 
+  test('does not reconcile from idle when fallback is in progress', async () => {
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+      description: 'fix bug',
+    });
+    expect(board.get('child-1')).toMatchObject({ state: 'running' });
+
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: (id) => id === 'parent-1',
+      isFallbackInProgress: (id) => id === 'child-1',
+    });
+
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'child-1' } },
+    });
+
+    // Job should still be running — not reconciled
+    expect(board.get('child-1')).toMatchObject({ state: 'running' });
+  });
+
+  test('reconciles from idle when fallback guard passes', async () => {
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+      description: 'fix bug',
+    });
+    expect(board.get('child-1')).toMatchObject({ state: 'running' });
+
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: (id) => id === 'parent-1',
+      // isFallbackInProgress returns false for child-1
+      isFallbackInProgress: () => false,
+    });
+
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'child-1' } },
+    });
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'reconciled',
+      terminalState: 'completed',
+    });
+  });
+
+  test('busy-after-idle from fallback re-prompt leaves job running', async () => {
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+      description: 'fix bug',
+    });
+    expect(board.get('child-1')).toMatchObject({
+      state: 'running',
+      timedOut: false,
+    });
+
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: () => false,
+      isFallbackInProgress: (id) => id === 'child-1',
+    });
+
+    // First idle (abort from fallback) — guarded, no reconciliation
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'child-1' } },
+    });
+    expect(board.get('child-1')).toMatchObject({ state: 'running' });
+
+    // Busy signal (fallback re-prompt) — updates lastLiveBusyAt
+    await hook.event({
+      event: {
+        type: 'session.status',
+        properties: { sessionID: 'child-1', status: { type: 'busy' } },
+      },
+    });
+    expect(board.get('child-1')).toMatchObject({ state: 'running' });
+
+    // Second idle (real completion) — fallback no longer in progress
+    const hook2 = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: () => false,
+      isFallbackInProgress: () => false,
+    });
+    await hook2.hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'child-1' } },
+    });
+    expect(board.get('child-1')).toMatchObject({
+      state: 'reconciled',
+      terminalState: 'completed',
+    });
+  });
+
+  test('cancelled job is not reconciled from idle', async () => {
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+      description: 'fix bug',
+    });
+    board.markCancelled('child-1', 'explicit cancel');
+    expect(board.get('child-1')).toMatchObject({ state: 'cancelled' });
+
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: () => false,
+    });
+
+    await hook.event({
+      event: { type: 'session.idle', properties: { sessionID: 'child-1' } },
+    });
+
+    // Should remain cancelled — idle does not override terminal state
+    const job = board.get('child-1');
+    expect(job?.state).toBe('cancelled');
+  });
+
+  test('idle via session.status idle path triggers reconciliation', async () => {
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+      description: 'fix bug',
+    });
+    expect(board.get('child-1')).toMatchObject({ state: 'running' });
+
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: (id) => id === 'parent-1',
+    });
+
+    await hook.event({
+      event: {
+        type: 'session.status',
+        properties: { sessionID: 'child-1', status: { type: 'idle' } },
+      },
+    });
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'reconciled',
+      terminalState: 'completed',
+    });
+  });
+
   test('parent deletion clears jobs and pending calls', async () => {
   test('parent deletion clears jobs and pending calls', async () => {
     const board = new BackgroundJobBoard();
     const board = new BackgroundJobBoard();
     const { hook } = createHook({ backgroundJobBoard: board });
     const { hook } = createHook({ backgroundJobBoard: board });

+ 35 - 19
src/hooks/task-session-manager/index.ts

@@ -84,6 +84,12 @@ export function createTaskSessionManagerHook(
     readContextMaxFiles?: number;
     readContextMaxFiles?: number;
     backgroundJobBoard?: BackgroundJobBoard;
     backgroundJobBoard?: BackgroundJobBoard;
     shouldManageSession: (sessionID: string) => boolean;
     shouldManageSession: (sessionID: string) => boolean;
+    /** Optional guard: when provided, idle events for a session that is
+     *  currently undergoing a foreground-fallback abort/re-prompt cycle
+     *  will NOT trigger idle reconciliation. Prevents marking a still-
+     *  active child job as completed when the session was aborted for
+     *  model fallback rather than natural completion. */
+    isFallbackInProgress?: (sessionID: string) => boolean;
   },
   },
 ) {
 ) {
   const backgroundJobBoard =
   const backgroundJobBoard =
@@ -574,6 +580,7 @@ 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;
+        const job = sessionId ? backgroundJobBoard.get(sessionId) : undefined;
         log('[task-session-manager] idle/status idle observed', {
         log('[task-session-manager] idle/status idle observed', {
           sessionID: sessionId,
           sessionID: sessionId,
           managesSession: sessionId
           managesSession: sessionId
@@ -582,9 +589,7 @@ export function createTaskSessionManagerHook(
           terminalJobsPending: sessionId
           terminalJobsPending: sessionId
             ? (terminalJobsInjectedByParent.get(sessionId)?.size ?? 0)
             ? (terminalJobsInjectedByParent.get(sessionId)?.size ?? 0)
             : 0,
             : 0,
-          runningJobForSession: sessionId
-            ? backgroundJobBoard.get(sessionId)?.state === 'running' || false
-            : false,
+          runningJobForSession: job?.state === 'running' || false,
         });
         });
         if (sessionId && options.shouldManageSession(sessionId)) {
         if (sessionId && options.shouldManageSession(sessionId)) {
           reconcileInjectedTerminalJobs(sessionId);
           reconcileInjectedTerminalJobs(sessionId);
@@ -594,22 +599,33 @@ export function createTaskSessionManagerHook(
         // Fallback: for background child sessions that go idle without
         // Fallback: for background child sessions that go idle without
         // an injected completion, reconcile the board entry since the
         // an injected completion, reconcile the board entry since the
         // session being idle is itself the completion signal.
         // session being idle is itself the completion signal.
-        if (sessionId) {
-          const job = backgroundJobBoard.get(sessionId);
-          if (job && job.state === 'running') {
-            log('[task-session-manager] reconciled running job from idle', {
-              sessionID: sessionId,
-              alias: job.alias,
-              parentSessionID: job.parentSessionID,
-            });
-            backgroundJobBoard.updateStatus({
-              taskID: sessionId,
-              state: 'completed',
-              resultSummary:
-                'Background task completed (reconciled from idle event)',
-            });
-            backgroundJobBoard.markReconciled(sessionId);
-          }
+        // Guard: skip when a foreground-fallback abort/re-prompt is in
+        // flight for this session — the idle is transient, not a real
+        // completion.
+        if (
+          job &&
+          sessionId &&
+          job.state === 'running' &&
+          !options.isFallbackInProgress?.(sessionId)
+        ) {
+          log('[task-session-manager] reconciled running job from idle', {
+            sessionID: sessionId,
+            alias: job.alias,
+            parentSessionID: job.parentSessionID,
+          });
+          backgroundJobBoard.updateStatus({
+            taskID: sessionId,
+            state: 'completed',
+            resultSummary:
+              'Background task completed (reconciled from idle event)',
+          });
+          backgroundJobBoard.markReconciled(sessionId);
+          taskContextTracker.pendingManagedTaskIds.delete(sessionId);
+          backgroundJobBoard.addContext(
+            sessionId,
+            taskContextTracker.contextFilesForPrompt(sessionId),
+          );
+          taskContextTracker.prune(backgroundJobBoard);
         }
         }
         return;
         return;
       }
       }

+ 2 - 0
src/index.ts

@@ -315,6 +315,8 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       backgroundJobBoard,
       backgroundJobBoard,
       shouldManageSession: (sessionID) =>
       shouldManageSession: (sessionID) =>
         sessionAgentMap.get(sessionID) === 'orchestrator',
         sessionAgentMap.get(sessionID) === 'orchestrator',
+      isFallbackInProgress: (sessionID) =>
+        foregroundFallback.isFallbackInProgress(sessionID),
     });
     });
     interviewManager = createInterviewManager(ctx, config);
     interviewManager = createInterviewManager(ctx, config);
     presetManager = createPresetManager(ctx, config);
     presetManager = createPresetManager(ctx, config);