Browse Source

fix(fallback): prevent foreground fallback abort from prematurely cancelling background tasks

When tryFallbackWithAbort() calls abortSessionWithTimeout(), the abort
triggers session.deleted which fires cleanup callbacks that:
1. Clear inProgress (defeating the isFallbackInProgress guard)
2. Drop the background job from the board

Both happen before execFallback can re-prompt with the fallback model.
The orchestrator then sees no record of the completed task and reports
it as cancelled — even though the oracle actually responded.

Changes:
- ForegroundFallbackManager: stop clearing inProgress in the
  onSessionDeleted callback. The finally blocks of tryFallback/
  tryFallbackWithAbort manage inProgress lifecycle.
- TaskSessionManager: guard backgroundJobBoard.drop() and
  clearParent() with the existing isFallbackInProgress option so
  jobs survive the abort/re-prompt cycle.

Closes #765
umi008 3 weeks ago
parent
commit
02ac280dd2

+ 24 - 0
src/hooks/foreground-fallback/index.test.ts

@@ -1255,6 +1255,30 @@ describe('ForegroundFallbackManager session.deleted', () => {
     // Triggered (dedup was cleared by deletion)
     expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
   });
+
+  test('does NOT clear inProgress when session.deleted fires', () => {
+    const coordinator = new SessionLifecycle(() => {});
+    const { client } = createMockClient();
+    const mgr = new ForegroundFallbackManager(
+      client,
+      makeChains(),
+      true,
+      3,
+      coordinator,
+    );
+
+    // Simulate: fallback is in progress
+    const sessionID = 'sess-inprog';
+    (mgr as any).inProgress.add(sessionID);
+    expect(mgr.isFallbackInProgress(sessionID)).toBe(true);
+
+    // Session deleted fires (as it does during abort in tryFallbackWithAbort)
+    coordinator.dispatchSessionDeleted(sessionID);
+
+    // inProgress must survive — the finally block of tryFallback/WithAbort
+    // manages it, not the session.deleted callback
+    expect(mgr.isFallbackInProgress(sessionID)).toBe(true);
+  });
 });
 
 // ---------------------------------------------------------------------------

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

@@ -241,7 +241,12 @@ export class ForegroundFallbackManager {
         this.sessionModel.delete(id);
         this.sessionAgent.delete(id);
         this.sessionTried.delete(id);
-        this.inProgress.delete(id);
+        // NOTE: inProgress is intentionally NOT cleared here —
+        // the finally blocks in tryFallback() and tryFallbackWithAbort()
+        // manage inProgress lifecycle. Clearing it here would make
+        // isFallbackInProgress() return false during the abort/re-prompt
+        // cycle, letting the task-session-manager treat the abort idle
+        // as a real completion and report a background task as cancelled.
         this.lastTrigger.delete(id);
         this.lastTriggerModel.delete(id);
         this.sessionRetries.delete(id);

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

@@ -2150,6 +2150,56 @@ describe('task-session-manager hook', () => {
     expect(board.get('child-1')).toMatchObject({ state: 'running' });
   });
 
+  test('does NOT drop job from board on session.deleted when fallback in progress', async () => {
+    const coordinator = new SessionLifecycle(() => {});
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'oracle',
+      description: 'architecture review',
+    });
+    expect(board.get('child-1')).toMatchObject({ state: 'running' });
+
+    createHook({
+      backgroundJobBoard: board,
+      coordinator,
+      isFallbackInProgress: (id) => id === 'child-1',
+    });
+
+    // Dispatch session.deleted while fallback is in progress
+    coordinator.dispatchSessionDeleted('child-1');
+
+    // Job must survive — the orchestrator needs to track it through the
+    // abort/re-prompt cycle
+    expect(board.get('child-1')).toBeDefined();
+    expect(board.get('child-1')).toMatchObject({ state: 'running' });
+  });
+
+  test('drops job from board on session.deleted when no fallback in progress', async () => {
+    const coordinator = new SessionLifecycle(() => {});
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-2',
+      parentSessionID: 'parent-1',
+      agent: 'oracle',
+      description: 'architecture review',
+    });
+    expect(board.get('child-2')).toMatchObject({ state: 'running' });
+
+    createHook({
+      backgroundJobBoard: board,
+      coordinator,
+      // isFallbackInProgress not set — no guard
+    });
+
+    // Dispatch session.deleted normally
+    coordinator.dispatchSessionDeleted('child-2');
+
+    // Job should be dropped
+    expect(board.get('child-2')).toBeUndefined();
+  });
+
   test('reconciles from idle when fallback guard passes', async () => {
     const board = new BackgroundJobBoard();
     board.registerLaunch({

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

@@ -128,8 +128,15 @@ export function createTaskSessionManagerHook(
 
   if (options.coordinator) {
     options.coordinator.onSessionDeleted((sessionId) => {
-      backgroundJobBoard.drop(sessionId);
-      backgroundJobBoard.clearParent(sessionId);
+      // During a foreground fallback abort/re-prompt cycle, the session
+      // is being torn down and immediately recreated with a fallback model.
+      // Dropping the job from the board here would make the orchestrator
+      // lose track of the task and report it as cancelled even though the
+      // oracle actually completed.
+      if (!options.isFallbackInProgress?.(sessionId)) {
+        backgroundJobBoard.drop(sessionId);
+        backgroundJobBoard.clearParent(sessionId);
+      }
       terminalJobsInjectedByParent.delete(sessionId);
       taskContextTracker.clearSession(sessionId);
       taskContextTracker.prune(backgroundJobBoard);