Просмотр исходного кода

fix: isolate terminal listener failures (#903)

Ulises Millan Guerrero 2 недель назад
Родитель
Сommit
4b8723e998

+ 20 - 0
src/utils/background-job-board.test.ts

@@ -532,6 +532,26 @@ describe('BackgroundJobBoard', () => {
     expect(listener).not.toHaveBeenCalled();
   });
 
+  test('throws in one listener does not prevent subsequent listeners from receiving notification', () => {
+    const board = new BackgroundJobBoard();
+    const order: string[] = [];
+    board.addTerminalStateListener(() => {
+      throw new Error('first listener failed');
+    });
+    board.addTerminalStateListener(() => {
+      order.push('second');
+    });
+    board.registerLaunch({
+      taskID: 'ses_1',
+      parentSessionID: 'parent-1',
+      agent: 'fixer',
+    });
+
+    board.updateStatus({ taskID: 'ses_1', state: 'completed' });
+
+    expect(order).toEqual(['second']);
+  });
+
   test('cancelled jobs ignore late non-cancelled terminal statuses', () => {
     const board = new BackgroundJobBoard();
     board.registerLaunch({

+ 9 - 1
src/utils/background-job-board.ts

@@ -5,6 +5,7 @@ import {
   formatSystemReminder,
 } from '../config/constants';
 import type { BackgroundJobStore } from './background-job-store';
+import { log } from './logger';
 import { parseTaskStatusOutput, type TaskOutputState } from './task';
 
 export interface ContextFile {
@@ -121,7 +122,14 @@ export class BackgroundJobBoard implements BackgroundJobStore {
 
   private notifyTerminalStateListeners(taskID: string): void {
     for (const listener of this.terminalStateListeners) {
-      listener(taskID);
+      try {
+        listener(taskID);
+      } catch (error) {
+        log('Board terminal state listener threw', {
+          taskID,
+          error: error instanceof Error ? error.message : String(error),
+        });
+      }
     }
   }
 

+ 26 - 0
src/utils/background-job-coordinator.test.ts

@@ -90,6 +90,32 @@ describe('BackgroundJobCoordinator', () => {
     expect(listener).not.toHaveBeenCalled();
   });
 
+  test('throws in one coordinator listener does not prevent subsequent listeners from receiving notification', () => {
+    const board = createMockBoard(true);
+    const coordinator = new BackgroundJobCoordinator(board);
+    const order: string[] = [];
+
+    coordinator.addTerminalStateListener(() => {
+      throw new Error('first listener failed');
+    });
+    coordinator.addTerminalStateListener(() => {
+      order.push('second');
+    });
+
+    // Defer the session
+    coordinator.deferIfRunning('ses_123');
+
+    // Simulate terminal state notification from board
+    board.getState.mockReturnValue('completed');
+    board.isRunning.mockReturnValue(false);
+
+    // Trigger handleTerminalState via board's listener callback
+    const boardListener = board.addTerminalStateListener.mock.calls[0]?.[0];
+    boardListener?.('ses_123');
+
+    expect(order).toEqual(['second']);
+  });
+
   test('full chain: board terminal → coordinator → listener for deferred job', () => {
     const board = new BackgroundJobBoard();
     const coordinator = new BackgroundJobCoordinator(board);

+ 9 - 1
src/utils/background-job-coordinator.ts

@@ -6,6 +6,7 @@ import type {
   ContextFile,
 } from './background-job-board';
 import type { BackgroundJobStore } from './background-job-store';
+import { log } from './logger';
 import type { TaskOutputState } from './task';
 
 type TerminalStateListener = (taskID: string) => void;
@@ -58,7 +59,14 @@ export class BackgroundJobCoordinator implements BackgroundJobStore {
     if (this.retryDeferredClose(taskID)) {
       // Notify listeners that session should close
       for (const listener of this.terminalStateListeners) {
-        listener(taskID);
+        try {
+          listener(taskID);
+        } catch (error) {
+          log('Coordinator terminal state listener threw', {
+            taskID,
+            error: error instanceof Error ? error.message : String(error),
+          });
+        }
       }
     }
   }