fix: stabilize multiplexer session pane lifecycle

src/multiplexer/session-manager.test.ts

@@ -255,7 +255,7 @@ describe('MultiplexerSessionManager', () => {
       expect(mockMultiplexer.closePane).not.toHaveBeenCalled();
     });
 
-    test('respawns pane on busy for known prior session', async () => {
+    test('respawns pane on later busy after idle close for resumable session', async () => {
       const ctx = createMockContext();
       const manager = new MultiplexerSessionManager(
         ctx,
@@ -308,6 +308,312 @@ describe('MultiplexerSessionManager', () => {
       expect(mockMultiplexer.closePane).toHaveBeenCalledTimes(1);
     });
 
+    test('respawns after in-flight idle close when busy resumes same session', async () => {
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+      const closeDeferred = createDeferred<boolean>();
+
+      mockMultiplexer.spawnPane
+        .mockResolvedValueOnce({
+          success: true,
+          paneId: 'p-close-race',
+        })
+        .mockResolvedValueOnce({
+          success: true,
+          paneId: 'p-close-race-resumed',
+        });
+      mockMultiplexer.closePane.mockImplementationOnce(
+        () => closeDeferred.promise,
+      );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: {
+            id: 'child-close-race',
+            parentID: 'parent-close-race',
+            title: 'Worker',
+          },
+        },
+      });
+
+      const idlePromise = manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-close-race',
+          status: { type: 'idle' },
+        },
+      });
+
+      await Promise.resolve();
+
+      const busyPromise = manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-close-race',
+          status: { type: 'busy' },
+        },
+      });
+
+      expect(mockMultiplexer.spawnPane).toHaveBeenCalledTimes(1);
+
+      closeDeferred.resolve(true);
+      await Promise.all([idlePromise, busyPromise]);
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledTimes(1);
+      expect(mockMultiplexer.spawnPane).toHaveBeenCalledTimes(2);
+      expect((manager as any).sessions.get('child-close-race')?.paneId).toBe(
+        'p-close-race-resumed',
+      );
+    });
+
+    test('does not respawn after in-flight close if session is deleted', async () => {
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+      const closeDeferred = createDeferred<boolean>();
+
+      mockMultiplexer.spawnPane
+        .mockResolvedValueOnce({
+          success: true,
+          paneId: 'p-delete-race',
+        })
+        .mockResolvedValueOnce({
+          success: true,
+          paneId: 'p-should-not-respawn',
+        });
+      mockMultiplexer.closePane.mockImplementationOnce(
+        () => closeDeferred.promise,
+      );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: {
+            id: 'child-delete-race',
+            parentID: 'parent-delete-race',
+            title: 'Worker',
+          },
+        },
+      });
+
+      const idlePromise = manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-delete-race',
+          status: { type: 'idle' },
+        },
+      });
+
+      await Promise.resolve();
+
+      const busyPromise = manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-delete-race',
+          status: { type: 'busy' },
+        },
+      });
+
+      const deletedPromise = manager.onSessionDeleted({
+        type: 'session.deleted',
+        properties: {
+          sessionID: 'child-delete-race',
+        },
+      });
+
+      closeDeferred.resolve(true);
+      await Promise.all([idlePromise, busyPromise, deletedPromise]);
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledTimes(1);
+      expect(mockMultiplexer.spawnPane).toHaveBeenCalledTimes(1);
+    });
+
+    test('deduplicates concurrent close requests for the same pane', async () => {
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+      const closeDeferred = createDeferred<boolean>();
+
+      mockMultiplexer.spawnPane.mockResolvedValueOnce({
+        success: true,
+        paneId: 'p-dedupe',
+      });
+      mockMultiplexer.closePane.mockImplementationOnce(
+        () => closeDeferred.promise,
+      );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: {
+            id: 'child-dedupe',
+            parentID: 'parent-dedupe',
+          },
+        },
+      });
+
+      const idleClose = manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-dedupe',
+          status: { type: 'idle' },
+        },
+      });
+
+      const deletedClose = manager.onSessionDeleted({
+        type: 'session.deleted',
+        properties: {
+          sessionID: 'child-dedupe',
+        },
+      });
+
+      await Promise.resolve();
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledTimes(1);
+
+      closeDeferred.resolve(true);
+      await Promise.all([idleClose, deletedClose]);
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledWith('p-dedupe');
+      expect(mockMultiplexer.closePane).toHaveBeenCalledTimes(1);
+    });
+
+    test('closes pane on session.deleted using info.id', async () => {
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+
+      mockMultiplexer.spawnPane.mockResolvedValueOnce({
+        success: true,
+        paneId: 'p-info-id',
+      });
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: {
+            id: 'child-info-id',
+            parentID: 'parent-info-id',
+          },
+        },
+      });
+
+      await manager.onSessionDeleted({
+        type: 'session.deleted',
+        properties: {
+          info: { id: 'child-info-id' },
+        },
+      });
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledWith('p-info-id');
+
+      await manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-info-id',
+          status: { type: 'busy' },
+        },
+      });
+
+      expect(mockMultiplexer.spawnPane).toHaveBeenCalledTimes(1);
+    });
+
+    test('closes pane returned by a stale spawn after session deleted', async () => {
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+      const spawnDeferred = createDeferred<{ success: true; paneId: string }>();
+
+      mockMultiplexer.spawnPane.mockImplementationOnce(
+        () => spawnDeferred.promise,
+      );
+
+      const createPromise = manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: {
+            id: 'child-stale-spawn',
+            parentID: 'parent-stale-spawn',
+          },
+        },
+      });
+
+      await Promise.resolve();
+
+      await manager.onSessionDeleted({
+        type: 'session.deleted',
+        properties: {
+          info: { id: 'child-stale-spawn' },
+        },
+      });
+
+      spawnDeferred.resolve({ success: true, paneId: 'p-stale-spawn' });
+      await createPromise;
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledWith('p-stale-spawn');
+      expect((manager as any).sessions.has('child-stale-spawn')).toBe(false);
+    });
+
+    test('does not let duplicate created event reopen a deleted pending spawn', async () => {
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+      const spawnDeferred = createDeferred<{ success: true; paneId: string }>();
+
+      mockMultiplexer.spawnPane.mockImplementationOnce(
+        () => spawnDeferred.promise,
+      );
+
+      const createEvent = {
+        type: 'session.created',
+        properties: {
+          info: {
+            id: 'child-duplicate-stale',
+            parentID: 'parent-duplicate-stale',
+          },
+        },
+      };
+
+      const firstCreate = manager.onSessionCreated(createEvent);
+
+      await Promise.resolve();
+
+      await manager.onSessionDeleted({
+        type: 'session.deleted',
+        properties: {
+          info: { id: 'child-duplicate-stale' },
+        },
+      });
+
+      await manager.onSessionCreated(createEvent);
+
+      expect(mockMultiplexer.spawnPane).toHaveBeenCalledTimes(1);
+
+      spawnDeferred.resolve({ success: true, paneId: 'p-duplicate-stale' });
+      await firstCreate;
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledWith(
+        'p-duplicate-stale',
+      );
+      expect((manager as any).sessions.has('child-duplicate-stale')).toBe(
+        false,
+      );
+    });
+
     test('does nothing on busy for unknown session', async () => {
       const ctx = createMockContext();
       const manager = new MultiplexerSessionManager(

src/multiplexer/session-manager.ts

@@ -41,6 +41,8 @@ interface SessionEvent {
   };
 }
 
+type CloseReason = 'idle' | 'deleted' | 'missing' | 'timeout';
+
 const SESSION_TIMEOUT_MS = 10 * 60 * 1000;
 const SESSION_MISSING_GRACE_MS = POLL_INTERVAL_BACKGROUND_MS * 3;
 
@@ -58,6 +60,8 @@ export class MultiplexerSessionManager {
   private sessions = new Map<string, TrackedSession>();
   private knownSessions = new Map<string, KnownSession>();
   private spawningSessions = new Set<string>();
+  private closingSessions = new Map<string, Promise<void>>();
+  private deletedSessions = new Set<string>();
   private pollInterval?: ReturnType<typeof setInterval>;
   private enabled = false;
 
@@ -95,6 +99,10 @@ export class MultiplexerSessionManager {
     const title = info.title ?? 'Subagent';
     const directory = info.directory ?? this.directory;
 
+    if (this.deletedSessions.has(sessionId)) {
+      return;
+    }
+
     this.knownSessions.set(sessionId, {
       parentId,
       title,
@@ -119,7 +127,7 @@ export class MultiplexerSessionManager {
         return;
       }
 
-      if (this.sessions.has(sessionId)) {
+      if (this.isDeletedOrClosing(sessionId) || this.sessions.has(sessionId)) {
         return;
       }
 
@@ -141,25 +149,36 @@ export class MultiplexerSessionManager {
           return { success: false, paneId: undefined };
         });
 
-      if (paneResult.success && paneResult.paneId) {
-        const now = Date.now();
-        this.sessions.set(sessionId, {
-          sessionId,
-          paneId: paneResult.paneId,
-          parentId,
-          title,
-          directory,
-          createdAt: now,
-          lastSeenAt: now,
-        });
-
-        log('[multiplexer-session-manager] pane spawned', {
-          sessionId,
-          paneId: paneResult.paneId,
-        });
+      if (!paneResult.success || !paneResult.paneId) return;
 
-        this.startPolling();
+      if (this.isDeletedOrClosing(sessionId)) {
+        await this.multiplexer.closePane(paneResult.paneId).catch((err) =>
+          log('[multiplexer-session-manager] closing stale spawned pane failed', {
+            sessionId,
+            paneId: paneResult.paneId,
+            error: String(err),
+          }),
+        );
+        return;
       }
+
+      const now = Date.now();
+      this.sessions.set(sessionId, {
+        sessionId,
+        paneId: paneResult.paneId,
+        parentId,
+        title,
+        directory,
+        createdAt: now,
+        lastSeenAt: now,
+      });
+
+      log('[multiplexer-session-manager] pane spawned', {
+        sessionId,
+        paneId: paneResult.paneId,
+      });
+
+      this.startPolling();
     } finally {
       this.spawningSessions.delete(sessionId);
     }
@@ -173,7 +192,7 @@ export class MultiplexerSessionManager {
     if (!sessionId) return;
 
     if (event.properties?.status?.type === 'idle') {
-      await this.closeSession(sessionId);
+      await this.closeSession(sessionId, 'idle');
       return;
     }
 
@@ -186,15 +205,14 @@ export class MultiplexerSessionManager {
     if (!this.enabled) return;
     if (event.type !== 'session.deleted') return;
 
-    const sessionId = event.properties?.sessionID;
+    const sessionId = this.getSessionId(event);
     if (!sessionId) return;
 
     log('[multiplexer-session-manager] session deleted, closing pane', {
       sessionId,
     });
 
-    await this.closeSession(sessionId);
-    this.knownSessions.delete(sessionId);
+    await this.closeSession(sessionId, 'deleted');
   }
 
   private startPolling(): void {
@@ -229,7 +247,8 @@ export class MultiplexerSessionManager {
       >;
 
       const now = Date.now();
-      const sessionsToClose: string[] = [];
+      const sessionsToClose: Array<{ sessionId: string; reason: CloseReason }> =
+        [];
 
       for (const [sessionId, tracked] of this.sessions.entries()) {
         const status = allStatuses[sessionId];
@@ -248,38 +267,84 @@ export class MultiplexerSessionManager {
         const isTimedOut = now - tracked.createdAt > SESSION_TIMEOUT_MS;
 
         if (isIdle || missingTooLong || isTimedOut) {
-          sessionsToClose.push(sessionId);
+          sessionsToClose.push({
+            sessionId,
+            reason: isIdle ? 'idle' : isTimedOut ? 'timeout' : 'missing',
+          });
         }
       }
 
-      for (const sessionId of sessionsToClose) {
-        await this.closeSession(sessionId);
+      for (const { sessionId, reason } of sessionsToClose) {
+        await this.closeSession(sessionId, reason);
       }
     } catch (err) {
       log('[multiplexer-session-manager] poll error', { error: String(err) });
     }
   }
 
-  private async closeSession(sessionId: string): Promise<void> {
+  private async closeSession(
+    sessionId: string,
+    reason: CloseReason,
+  ): Promise<void> {
+    if (reason === 'deleted') {
+      this.markSessionDeleted(sessionId);
+    }
+
+    const existingClose = this.closingSessions.get(sessionId);
+    if (existingClose) return existingClose;
+
     const tracked = this.sessions.get(sessionId);
     if (!tracked || !this.multiplexer) return;
 
+    this.sessions.delete(sessionId);
+
     log('[multiplexer-session-manager] closing session pane', {
       sessionId,
       paneId: tracked.paneId,
+      reason,
     });
 
-    await this.multiplexer.closePane(tracked.paneId);
-    this.sessions.delete(sessionId);
+    const closePromise: Promise<void> = this.multiplexer
+      .closePane(tracked.paneId)
+      .then(() => undefined)
+      .catch((err) =>
+        log('[multiplexer-session-manager] failed to close session pane', {
+          sessionId,
+          paneId: tracked.paneId,
+          reason,
+          error: String(err),
+        }),
+      )
+      .finally(() => {
+        this.closingSessions.delete(sessionId);
+
+        if (this.sessions.size === 0) {
+          this.stopPolling();
+        }
+      });
 
-    if (this.sessions.size === 0) {
-      this.stopPolling();
-    }
+    this.closingSessions.set(sessionId, closePromise);
+    await closePromise;
+  }
+
+  private markSessionDeleted(sessionId: string): void {
+    this.deletedSessions.add(sessionId);
+    this.knownSessions.delete(sessionId);
   }
 
   private async respawnIfKnown(sessionId: string): Promise<void> {
     if (!this.enabled || !this.multiplexer) return;
-    if (this.isTrackedOrSpawning(sessionId)) return;
+    if (this.deletedSessions.has(sessionId)) return;
+
+    const closing = this.closingSessions.get(sessionId);
+    if (closing) {
+      await closing;
+      if (this.deletedSessions.has(sessionId)) return;
+    }
+
+    if (this.isTrackedOrSpawning(sessionId)) {
+      return;
+    }
 
     const known = this.knownSessions.get(sessionId);
     if (!known) return;
@@ -299,7 +364,9 @@ export class MultiplexerSessionManager {
         return;
       }
 
-      if (this.sessions.has(sessionId)) return;
+      if (this.sessions.has(sessionId) || this.isDeletedOrClosing(sessionId)) {
+        return;
+      }
 
       log(
         '[multiplexer-session-manager] child session busy again, respawning pane',
@@ -321,6 +388,17 @@ export class MultiplexerSessionManager {
 
       if (!paneResult.success || !paneResult.paneId) return;
 
+      if (this.isDeletedOrClosing(sessionId)) {
+        await this.multiplexer.closePane(paneResult.paneId).catch((err) =>
+          log('[multiplexer-session-manager] closing stale respawned pane failed', {
+            sessionId,
+            paneId: paneResult.paneId,
+            error: String(err),
+          }),
+        );
+        return;
+      }
+
       const now = Date.now();
       this.sessions.set(sessionId, {
         sessionId,
@@ -347,9 +425,23 @@ export class MultiplexerSessionManager {
     return this.sessions.has(sessionId) || this.spawningSessions.has(sessionId);
   }
 
+  private isDeletedOrClosing(sessionId: string): boolean {
+    return (
+      this.deletedSessions.has(sessionId) || this.closingSessions.has(sessionId)
+    );
+  }
+
+  private getSessionId(event: SessionEvent): string | undefined {
+    return event.properties?.info?.id ?? event.properties?.sessionID;
+  }
+
   async cleanup(): Promise<void> {
     this.stopPolling();
 
+    if (this.closingSessions.size > 0) {
+      await Promise.all(this.closingSessions.values());
+    }
+
     if (this.sessions.size > 0 && this.multiplexer) {
       log('[multiplexer-session-manager] closing all panes', {
         count: this.sessions.size,
@@ -369,6 +461,8 @@ export class MultiplexerSessionManager {
 
     this.knownSessions.clear();
     this.spawningSessions.clear();
+    this.deletedSessions.clear();
+    this.closingSessions.clear();
 
     log('[multiplexer-session-manager] cleanup complete');
   }