Browse Source

refactor(multiplexer): skip redundant lifecycle check on coordinator close path

- Replace BackgroundJobReader interface with Pick<BackgroundJobStore, ...> (removes duplicate interface)

- Add skipPolicyCheck param to closeSession; closeSessionFromCoordinator passes true since coordinator already vetted

- Add full-chain integration test: board terminal -> coordinator -> listener for deferred jobs
Michael Henke 1 month ago
parent
commit
4c55fb3385
2 changed files with 43 additions and 12 deletions
  1. 13 11
      src/multiplexer/session-manager.ts
  2. 30 1
      src/utils/background-job-coordinator.test.ts

+ 13 - 11
src/multiplexer/session-manager.ts

@@ -7,17 +7,13 @@ import {
   type Multiplexer,
 } from '../multiplexer';
 import type { BackgroundJobState } from '../utils/background-job-board';
+import type { BackgroundJobStore } from '../utils/background-job-store';
 import { log } from '../utils/logger';
 
-/**
- * Minimal interface for reading background job state.
- * Both BackgroundJobBoard and BackgroundJobCoordinator satisfy this.
- */
-interface BackgroundJobReader {
-  getState(sessionId: string): BackgroundJobState | undefined;
-  deferIfRunning(sessionId: string): boolean;
-  clearDeferredClose(sessionId: string): void;
-}
+type BackgroundJobReader = Pick<
+  BackgroundJobStore,
+  'getState' | 'deferIfRunning' | 'clearDeferredClose'
+>;
 
 interface TrackedSession {
   sessionId: string;
@@ -411,6 +407,7 @@ export class MultiplexerSessionManager {
   private async closeSession(
     sessionId: string,
     reason: CloseReason,
+    skipPolicyCheck = false,
   ): Promise<void> {
     if (reason === 'deleted') {
       this.knownSessions.delete(sessionId);
@@ -452,7 +449,11 @@ export class MultiplexerSessionManager {
       });
     }
 
-    if (reason === 'idle' && !this.shouldCloseNow(sessionId)) {
+    if (
+      reason === 'idle' &&
+      !skipPolicyCheck &&
+      !this.shouldCloseNow(sessionId)
+    ) {
       log(
         '[multiplexer-session-manager] close skipped; background job running',
         {
@@ -621,7 +622,8 @@ export class MultiplexerSessionManager {
 
   async closeSessionFromCoordinator(sessionId: string): Promise<void> {
     if (!this.enabled) return;
-    await this.closeSession(sessionId, 'idle');
+    // Coordinator already vetted lifecycle policy; skip re-check
+    await this.closeSession(sessionId, 'idle', true);
   }
 
   async cleanup(): Promise<void> {

+ 30 - 1
src/utils/background-job-coordinator.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, mock, test } from 'bun:test';
+import { BackgroundJobBoard } from './background-job-board';
 import { BackgroundJobCoordinator } from './background-job-coordinator';
 
 function createMockBoard(isRunning = false) {
@@ -7,7 +8,6 @@ function createMockBoard(isRunning = false) {
     getState: mock(() => (isRunning ? 'running' : 'completed')),
     addTerminalStateListener: mock(() => {}),
     removeTerminalStateListener: mock(() => {}),
-    // ... other methods as needed
   } as any;
 }
 
@@ -89,4 +89,33 @@ describe('BackgroundJobCoordinator', () => {
 
     expect(listener).not.toHaveBeenCalled();
   });
+
+  test('full chain: board terminal → coordinator → listener for deferred job', () => {
+    const board = new BackgroundJobBoard();
+    const coordinator = new BackgroundJobCoordinator(board);
+    const listener = mock(() => {});
+    coordinator.addTerminalStateListener(listener);
+
+    // Register and start a job
+    board.registerLaunch({
+      taskID: 'full-chain-test',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+    });
+    board.updateStatus({
+      taskID: 'full-chain-test',
+      state: 'running',
+    });
+
+    // Defer close while job is running
+    expect(coordinator.deferIfRunning('full-chain-test')).toBe(false);
+
+    // Transition to completed — board fires listener, coordinator re-checks
+    board.updateStatus({
+      taskID: 'full-chain-test',
+      state: 'completed',
+    });
+
+    expect(listener).toHaveBeenCalledWith('full-chain-test');
+  });
 });