Browse Source

fix multiplexer duplicate pane lifecycle

Alvin Unreal 2 months ago
parent
commit
fe3d9198e6

+ 148 - 0
src/multiplexer/session-manager.test.ts

@@ -58,6 +58,7 @@ function createDeferred<T>() {
 
 describe('MultiplexerSessionManager', () => {
   const realDateNow = Date.now;
+  const originalChildEnv = process.env.OMOS_MULTIPLEXER_CHILD;
 
   beforeEach(() => {
     mockMultiplexer.spawnPane.mockReset();
@@ -70,10 +71,16 @@ describe('MultiplexerSessionManager', () => {
     mockMultiplexer.isInsideSession.mockReset();
     mockMultiplexer.isInsideSession.mockReturnValue(true);
     Date.now = realDateNow;
+    delete process.env.OMOS_MULTIPLEXER_CHILD;
   });
 
   afterEach(() => {
     Date.now = realDateNow;
+    if (originalChildEnv === undefined) {
+      delete process.env.OMOS_MULTIPLEXER_CHILD;
+    } else {
+      process.env.OMOS_MULTIPLEXER_CHILD = originalChildEnv;
+    }
   });
 
   describe('constructor', () => {
@@ -85,6 +92,22 @@ describe('MultiplexerSessionManager', () => {
       );
       expect(manager).toBeDefined();
     });
+
+    test('disables pane spawning inside spawned child attach panes', async () => {
+      process.env.OMOS_MULTIPLEXER_CHILD = '1';
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: { info: { id: 'child-env', parentID: 'parent-env' } },
+      });
+
+      expect(mockMultiplexer.spawnPane).not.toHaveBeenCalled();
+    });
   });
 
   describe('onSessionCreated', () => {
@@ -213,6 +236,30 @@ describe('MultiplexerSessionManager', () => {
 
       expect(mockMultiplexer.spawnPane).toHaveBeenCalledTimes(1);
     });
+
+    test('does not respawn known sessions on replayed create events', async () => {
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: { id: 'child-known', parentID: 'parent-known' },
+        },
+      });
+      await (manager as any).closeSession('child-known', 'idle');
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: { id: 'child-known', parentID: 'parent-known' },
+        },
+      });
+
+      expect(mockMultiplexer.spawnPane).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('polling and closure', () => {
@@ -317,6 +364,107 @@ describe('MultiplexerSessionManager', () => {
       expect(mockMultiplexer.closePane).not.toHaveBeenCalled();
     });
 
+    test('busy during spawn is remembered so later idle can close', async () => {
+      const ctx = createMockContext();
+      let now = 1_000;
+      Date.now = () => now;
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+      const deferred = createDeferred<{ success: true; paneId: string }>();
+      mockMultiplexer.spawnPane.mockImplementationOnce(() => deferred.promise);
+
+      const createPromise = manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: { id: 'child-spawn-busy', parentID: 'parent-spawn-busy' },
+        },
+      });
+      await Promise.resolve();
+
+      await manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-spawn-busy',
+          status: { type: 'busy' },
+        },
+      });
+
+      deferred.resolve({ success: true, paneId: 'p-spawn-busy' });
+      await createPromise;
+
+      ctx.client.session.status.mockResolvedValue({
+        data: { 'child-spawn-busy': { type: 'idle' } },
+      });
+      now += 16_000;
+      await (manager as any).pollSessions();
+      now += 7_500;
+      await (manager as any).pollSessions();
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledWith('p-spawn-busy');
+    });
+
+    test('persistent pre-busy idle eventually closes after grace', async () => {
+      const ctx = createMockContext();
+      let now = 1_000;
+      Date.now = () => now;
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: { info: { id: 'child-pre-busy', parentID: 'parent' } },
+      });
+      await manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-pre-busy',
+          status: { type: 'idle' },
+        },
+      });
+
+      now += 16_000;
+      await manager.onSessionStatus({
+        type: 'session.status',
+        properties: {
+          sessionID: 'child-pre-busy',
+          status: { type: 'idle' },
+        },
+      });
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalled();
+    });
+
+    test('handles session.idle events like idle status events', async () => {
+      const ctx = createMockContext();
+      let now = 1_000;
+      Date.now = () => now;
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: { info: { id: 'child-idle-event', parentID: 'parent' } },
+      });
+      await manager.onSessionStatus({
+        type: 'session.idle',
+        properties: { sessionID: 'child-idle-event' },
+      });
+
+      now += 16_000;
+      await manager.onSessionStatus({
+        type: 'session.idle',
+        properties: { sessionID: 'child-idle-event' },
+      });
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalled();
+    });
+
     test('does not close on missing status during initial grace period', async () => {
       const ctx = createMockContext();
       let now = 1_000;

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

@@ -64,6 +64,8 @@ export class MultiplexerSessionManager {
   private sessions = new Map<string, TrackedSession>();
   private knownSessions = new Map<string, KnownSession>();
   private spawningSessions = new Set<string>();
+  private pendingBusySessions = new Set<string>();
+  private pendingIdleSessions = new Map<string, number>();
   private closingSessions = new Map<string, Promise<void>>();
   private pollInterval?: ReturnType<typeof setInterval>;
   private enabled = false;
@@ -77,9 +79,9 @@ export class MultiplexerSessionManager {
 
     this.multiplexer = getMultiplexer(config);
     this.enabled =
+      process.env.OMOS_MULTIPLEXER_CHILD !== '1' &&
       config.type !== 'none' &&
-      this.multiplexer !== null &&
-      this.multiplexer.isInsideSession();
+      this.multiplexer?.isInsideSession() === true;
 
     log('[multiplexer-session-manager] initialized', {
       enabled: this.enabled,
@@ -109,6 +111,18 @@ export class MultiplexerSessionManager {
       return;
     }
 
+    if (this.knownSessions.has(sessionId)) {
+      this.knownSessions.set(sessionId, {
+        parentId,
+        title,
+        directory,
+      });
+      log('[multiplexer-session-manager] known session create ignored', {
+        sessionId,
+      });
+      return;
+    }
+
     const closing = this.closingSessions.get(sessionId);
     if (closing) await closing;
 
@@ -173,6 +187,7 @@ export class MultiplexerSessionManager {
       }
 
       const now = Date.now();
+      const pendingIdleSince = this.pendingIdleSessions.get(sessionId);
       this.sessions.set(sessionId, {
         sessionId,
         paneId: paneResult.paneId,
@@ -181,8 +196,11 @@ export class MultiplexerSessionManager {
         directory,
         createdAt: now,
         lastSeenAt: now,
-        hasSeenBusy: false,
+        hasSeenBusy: this.pendingBusySessions.has(sessionId),
+        idleSince: pendingIdleSince,
       });
+      this.pendingBusySessions.delete(sessionId);
+      this.pendingIdleSessions.delete(sessionId);
 
       log('[multiplexer-session-manager] pane spawned', {
         sessionId,
@@ -197,12 +215,17 @@ export class MultiplexerSessionManager {
 
   async onSessionStatus(event: SessionEvent): Promise<void> {
     if (!this.enabled) return;
-    if (event.type !== 'session.status') return;
+    if (event.type !== 'session.status' && event.type !== 'session.idle') {
+      return;
+    }
 
     const sessionId = event.properties?.sessionID;
     if (!sessionId) return;
 
-    if (event.properties?.status?.type === 'idle') {
+    if (
+      event.type === 'session.idle' ||
+      event.properties?.status?.type === 'idle'
+    ) {
       await this.closeIfIdleConfirmed(sessionId, Date.now());
       return;
     }
@@ -309,6 +332,8 @@ export class MultiplexerSessionManager {
   ): Promise<void> {
     if (reason === 'deleted') {
       this.knownSessions.delete(sessionId);
+      this.pendingBusySessions.delete(sessionId);
+      this.pendingIdleSessions.delete(sessionId);
     }
 
     const existingClose = this.closingSessions.get(sessionId);
@@ -350,7 +375,15 @@ export class MultiplexerSessionManager {
     now: number,
   ): Promise<void> {
     const tracked = this.sessions.get(sessionId);
-    if (!tracked) return;
+    if (!tracked) {
+      if (
+        this.spawningSessions.has(sessionId) ||
+        this.knownSessions.has(sessionId)
+      ) {
+        this.pendingIdleSessions.set(sessionId, now);
+      }
+      return;
+    }
 
     if (this.markIdleAndCheck(tracked, now)) {
       await this.closeSession(sessionId, 'idle');
@@ -361,13 +394,6 @@ export class MultiplexerSessionManager {
     tracked.lastSeenAt = now;
     tracked.missingSince = undefined;
 
-    if (!tracked.hasSeenBusy) {
-      log('[multiplexer-session-manager] idle ignored before busy observed', {
-        sessionId: tracked.sessionId,
-      });
-      return false;
-    }
-
     if (!tracked.idleSince) {
       tracked.idleSince = now;
       log('[multiplexer-session-manager] idle observed, waiting to confirm', {
@@ -389,6 +415,12 @@ export class MultiplexerSessionManager {
       tracked.idleSince = undefined;
       tracked.missingSince = undefined;
       tracked.lastSeenAt = now;
+    } else if (
+      this.spawningSessions.has(sessionId) ||
+      this.knownSessions.has(sessionId)
+    ) {
+      this.pendingBusySessions.add(sessionId);
+      this.pendingIdleSessions.delete(sessionId);
     }
   }
 

+ 1 - 0
src/multiplexer/tmux/index.ts

@@ -58,6 +58,7 @@ export class TmuxMultiplexer implements Multiplexer {
       const quotedSessionId = quoteShellArg(sessionId);
 
       const opencodeCmd = [
+        'OMOS_MULTIPLEXER_CHILD=1',
         'opencode',
         'attach',
         quotedUrl,

+ 1 - 0
src/multiplexer/zellij/index.ts

@@ -479,6 +479,7 @@ function buildOpencodeAttachCommand(
   directory: string,
 ): string {
   return [
+    'OMOS_MULTIPLEXER_CHILD=1',
     'opencode',
     'attach',
     quoteShellArg(serverUrl),