Browse Source

fix: address code review findings

- Remove unused isRunning from BackgroundJobReader interface
- Remove redundant clearDeferredClose on session.deleted path
- Rename closeSessionFromCoordinator parameter from taskID to sessionId
Michael Henke 3 weeks ago
parent
commit
7fbc86b189
2 changed files with 18 additions and 20 deletions
  1. 16 16
      src/multiplexer/session-manager.test.ts
  2. 2 4
      src/multiplexer/session-manager.ts

+ 16 - 16
src/multiplexer/session-manager.test.ts

@@ -435,8 +435,8 @@ describe('MultiplexerSessionManager', () => {
         defaultMultiplexerConfig,
         coordinator,
       );
-      coordinator.addTerminalStateListener((taskID) => {
-        void manager.closeSessionFromCoordinator(taskID);
+      coordinator.addTerminalStateListener((sessionId) => {
+        void manager.closeSessionFromCoordinator(sessionId);
       });
 
       await manager.onSessionCreated({
@@ -501,8 +501,8 @@ describe('MultiplexerSessionManager', () => {
           defaultMultiplexerConfig,
           coordinator,
         );
-        coordinator.addTerminalStateListener((taskID) => {
-          void manager.closeSessionFromCoordinator(taskID);
+        coordinator.addTerminalStateListener((sessionId) => {
+          void manager.closeSessionFromCoordinator(sessionId);
         });
 
         await manager.onSessionCreated({
@@ -540,8 +540,8 @@ describe('MultiplexerSessionManager', () => {
         defaultMultiplexerConfig,
         coordinator,
       );
-      coordinator.addTerminalStateListener((taskID) => {
-        void manager.closeSessionFromCoordinator(taskID);
+      coordinator.addTerminalStateListener((sessionId) => {
+        void manager.closeSessionFromCoordinator(sessionId);
       });
 
       await manager.onSessionCreated({
@@ -581,8 +581,8 @@ describe('MultiplexerSessionManager', () => {
         defaultMultiplexerConfig,
         coordinator,
       );
-      coordinator.addTerminalStateListener((taskID) => {
-        void manager.closeSessionFromCoordinator(taskID);
+      coordinator.addTerminalStateListener((sessionId) => {
+        void manager.closeSessionFromCoordinator(sessionId);
       });
 
       await manager.onSessionCreated({
@@ -618,8 +618,8 @@ describe('MultiplexerSessionManager', () => {
         defaultMultiplexerConfig,
         coordinator,
       );
-      coordinator.addTerminalStateListener((taskID) => {
-        void manager.closeSessionFromCoordinator(taskID);
+      coordinator.addTerminalStateListener((sessionId) => {
+        void manager.closeSessionFromCoordinator(sessionId);
       });
 
       await manager.onSessionCreated({
@@ -661,8 +661,8 @@ describe('MultiplexerSessionManager', () => {
         defaultMultiplexerConfig,
         coordinator,
       );
-      coordinator.addTerminalStateListener((taskID) => {
-        void manager.closeSessionFromCoordinator(taskID);
+      coordinator.addTerminalStateListener((sessionId) => {
+        void manager.closeSessionFromCoordinator(sessionId);
       });
 
       await manager.onSessionCreated({
@@ -756,8 +756,8 @@ describe('MultiplexerSessionManager', () => {
         defaultMultiplexerConfig,
         coordinator,
       );
-      coordinator.addTerminalStateListener((taskID) => {
-        void manager.closeSessionFromCoordinator(taskID);
+      coordinator.addTerminalStateListener((sessionId) => {
+        void manager.closeSessionFromCoordinator(sessionId);
       });
 
       await manager.onSessionCreated({
@@ -819,8 +819,8 @@ describe('MultiplexerSessionManager', () => {
         defaultMultiplexerConfig,
         coordinator,
       );
-      coordinator.addTerminalStateListener((taskID) => {
-        void manager.closeSessionFromCoordinator(taskID);
+      coordinator.addTerminalStateListener((sessionId) => {
+        void manager.closeSessionFromCoordinator(sessionId);
       });
 
       await manager.onSessionCreated({

+ 2 - 4
src/multiplexer/session-manager.ts

@@ -15,7 +15,6 @@ import { log } from '../utils/logger';
  */
 interface BackgroundJobReader {
   getState(sessionId: string): BackgroundJobState | undefined;
-  isRunning(sessionId: string): boolean;
   deferIfRunning(sessionId: string): boolean;
   clearDeferredClose(sessionId: string): void;
 }
@@ -319,7 +318,6 @@ export class MultiplexerSessionManager {
       backgroundJobState: this.backgroundJobState(sessionId),
     });
 
-    this.backgroundJobBoard?.clearDeferredClose(sessionId);
     await this.closeSession(sessionId, 'deleted');
   }
 
@@ -621,9 +619,9 @@ export class MultiplexerSessionManager {
     return this.backgroundJobBoard?.deferIfRunning(sessionId) ?? true;
   }
 
-  async closeSessionFromCoordinator(taskID: string): Promise<void> {
+  async closeSessionFromCoordinator(sessionId: string): Promise<void> {
     if (!this.enabled) return;
-    await this.closeSession(taskID, 'idle');
+    await this.closeSession(sessionId, 'idle');
   }
 
   async cleanup(): Promise<void> {