Browse Source

Fix multiplexer background pane lifecycle

alvinreal 2 months ago
parent
commit
acb1452d85
3 changed files with 153 additions and 34 deletions
  1. 2 2
      src/index.ts
  2. 114 24
      src/multiplexer/session-manager.test.ts
  3. 37 8
      src/multiplexer/session-manager.ts

+ 2 - 2
src/index.ts

@@ -254,12 +254,14 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
 
     mcps = createBuiltinMcps(config.disabled_mcps, config.websearch);
     webfetch = createWebfetchTool(ctx);
+    backgroundJobBoard = new BackgroundJobBoard();
 
     // Initialize MultiplexerSessionManager to handle OpenCode's built-in
     // Task tool sessions
     multiplexerSessionManager = new MultiplexerSessionManager(
       ctx,
       multiplexerConfig,
+      backgroundJobBoard,
     );
 
     // Initialize auto-update checker hook
@@ -299,8 +301,6 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
         Object.keys(runtimeChains).length > 0,
     );
 
-    backgroundJobBoard = new BackgroundJobBoard();
-
     // Initialize todo-continuation hook (opt-in auto-continue for
     // incomplete todos)
     todoContinuationHook = createTodoContinuationHook(ctx, {

+ 114 - 24
src/multiplexer/session-manager.test.ts

@@ -1,4 +1,5 @@
 import { afterEach, beforeEach, describe, expect, mock, test } from 'bun:test';
+import { BackgroundJobBoard } from '../utils/background-job-board';
 import {
   MultiplexerSessionManager,
   resetMultiplexerSessionManagerState,
@@ -390,11 +391,51 @@ describe('MultiplexerSessionManager', () => {
       expect(mockMultiplexer.closePane).not.toHaveBeenCalled();
     });
 
-    test('does not close missing session that was never seen in status', async () => {
+    test('keeps background child pane open while status is running until deleted', async () => {
+      const ctx = createMockContext();
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+
+      mockMultiplexer.spawnPane.mockResolvedValueOnce({
+        success: true,
+        paneId: 'p-background-child',
+      });
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: {
+          info: {
+            id: 'background-child',
+            parentID: 'parent-1',
+            title: 'Background Worker',
+          },
+        },
+      });
+
+      setMockSessionStatuses({ 'background-child': { type: 'running' } });
+      await (manager as any).pollSessions();
+      await (manager as any).pollSessions();
+
+      expect(mockMultiplexer.closePane).not.toHaveBeenCalled();
+
+      await manager.onSessionDeleted({
+        type: 'session.deleted',
+        properties: { info: { id: 'background-child' } },
+      });
+
+      expect(mockMultiplexer.closePane).toHaveBeenCalledTimes(1);
+      expect(mockMultiplexer.closePane).toHaveBeenCalledWith(
+        'p-background-child',
+      );
+    });
+
+    test('does not close long-running pane based on age alone', async () => {
       const ctx = createMockContext();
       mockMultiplexer.spawnPane.mockResolvedValue({
         success: true,
-        paneId: 'p-never-seen',
+        paneId: 'p-long-running',
       });
       const manager = new MultiplexerSessionManager(
         ctx,
@@ -403,56 +444,105 @@ describe('MultiplexerSessionManager', () => {
 
       await manager.onSessionCreated({
         type: 'session.created',
-        properties: { info: { id: 'never-seen', parentID: 'p1' } },
+        properties: { info: { id: 'long-running', parentID: 'p1' } },
       });
 
-      const tracked = (manager as any).sessions.get('never-seen');
-      tracked.missingSince = Date.now() - 60_000;
+      const tracked = (manager as any).sessions.get('long-running');
+      tracked.createdAt = Date.now() - 11 * 60 * 1000;
 
-      setMockSessionStatuses({});
+      setMockSessionStatuses({ 'long-running': { type: 'running' } });
       await (manager as any).pollSessions();
 
       expect(mockMultiplexer.closePane).not.toHaveBeenCalled();
     });
 
-    test('keeps background child pane open while status is running until deleted', async () => {
+    test('keeps missing running background job pane open', async () => {
       const ctx = createMockContext();
+      const board = new BackgroundJobBoard();
+      board.registerLaunch({
+        taskID: 'running-background-job',
+        parentSessionID: 'parent-1',
+        agent: 'explorer',
+      });
+      mockMultiplexer.spawnPane.mockResolvedValue({
+        success: true,
+        paneId: 'p-running-background-job',
+      });
       const manager = new MultiplexerSessionManager(
         ctx,
         defaultMultiplexerConfig,
+        board,
       );
 
-      mockMultiplexer.spawnPane.mockResolvedValueOnce({
-        success: true,
-        paneId: 'p-background-child',
-      });
-
       await manager.onSessionCreated({
         type: 'session.created',
         properties: {
-          info: {
-            id: 'background-child',
-            parentID: 'parent-1',
-            title: 'Background Worker',
-          },
+          info: { id: 'running-background-job', parentID: 'parent-1' },
         },
       });
 
-      setMockSessionStatuses({ 'background-child': { type: 'running' } });
-      await (manager as any).pollSessions();
+      const tracked = (manager as any).sessions.get('running-background-job');
+      tracked.missingSince = Date.now() - 60_000;
+
+      setMockSessionStatuses({});
       await (manager as any).pollSessions();
 
       expect(mockMultiplexer.closePane).not.toHaveBeenCalled();
+    });
 
-      await manager.onSessionDeleted({
-        type: 'session.deleted',
-        properties: { info: { id: 'background-child' } },
+    test('closes never-seen pane when no running background job exists', async () => {
+      const ctx = createMockContext();
+      mockMultiplexer.spawnPane.mockResolvedValue({
+        success: true,
+        paneId: 'p-never-seen-orphan',
       });
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
+      );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: { info: { id: 'never-seen-orphan', parentID: 'p1' } },
+      });
+
+      const tracked = (manager as any).sessions.get('never-seen-orphan');
+      tracked.missingSince = Date.now() - 60_000;
+
+      setMockSessionStatuses({});
+      await (manager as any).pollSessions();
 
-      expect(mockMultiplexer.closePane).toHaveBeenCalledTimes(1);
       expect(mockMultiplexer.closePane).toHaveBeenCalledWith(
-        'p-background-child',
+        'p-never-seen-orphan',
+      );
+    });
+
+    test('ignores empty session status response without closing panes', async () => {
+      const ctx = createMockContext();
+      mockMultiplexer.spawnPane.mockResolvedValue({
+        success: true,
+        paneId: 'p-empty-status',
+      });
+      const manager = new MultiplexerSessionManager(
+        ctx,
+        defaultMultiplexerConfig,
       );
+
+      await manager.onSessionCreated({
+        type: 'session.created',
+        properties: { info: { id: 'empty-status', parentID: 'p1' } },
+      });
+
+      const tracked = (manager as any).sessions.get('empty-status');
+      tracked.seenInStatus = true;
+      tracked.missingSince = Date.now() - 60_000;
+      mockFetch.mockImplementationOnce(
+        async () => new Response('', { status: 200 }),
+      );
+
+      await (manager as any).pollSessions();
+
+      expect(mockMultiplexer.closePane).not.toHaveBeenCalled();
     });
 
     test('keeps missing cleanup for sessions previously seen in status', async () => {

+ 37 - 8
src/multiplexer/session-manager.ts

@@ -6,6 +6,7 @@ import {
   isServerRunning,
   type Multiplexer,
 } from '../multiplexer';
+import type { BackgroundJobBoard } from '../utils/background-job-board';
 import { log } from '../utils/logger';
 
 interface TrackedSession {
@@ -47,9 +48,8 @@ interface SessionEvent {
   };
 }
 
-type CloseReason = 'idle' | 'deleted' | 'missing' | 'timeout';
+type CloseReason = 'idle' | 'deleted' | 'missing';
 
-const SESSION_TIMEOUT_MS = 10 * 60 * 1000;
 const SESSION_MISSING_GRACE_MS = POLL_INTERVAL_BACKGROUND_MS * 3;
 const SHARED_STATE_KEY = Symbol.for(
   'oh-my-opencode-slim.multiplexer-session-manager.state',
@@ -96,7 +96,11 @@ export class MultiplexerSessionManager {
   private pollInterval?: ReturnType<typeof setInterval>;
   private enabled = false;
 
-  constructor(ctx: PluginInput, config: MultiplexerConfig) {
+  constructor(
+    ctx: PluginInput,
+    config: MultiplexerConfig,
+    private readonly backgroundJobBoard?: BackgroundJobBoard,
+  ) {
     const sharedState = getSharedState();
     this.sessions = sharedState.sessions;
     this.knownSessions = sharedState.knownSessions;
@@ -339,15 +343,27 @@ export class MultiplexerSessionManager {
         }
 
         const missingTooLong =
-          tracked.seenInStatus &&
           !!tracked.missingSince &&
           now - tracked.missingSince >= SESSION_MISSING_GRACE_MS;
-        const isTimedOut = now - tracked.createdAt > SESSION_TIMEOUT_MS;
+        const shouldKeepRunningBackgroundJob =
+          missingTooLong && this.isRunningBackgroundJob(sessionId);
+        if (isIdle || missingTooLong) {
+          if (shouldKeepRunningBackgroundJob) {
+            log(
+              '[multiplexer-session-manager] keeping running background pane',
+              {
+                instanceId: this.instanceId,
+                sessionId,
+                paneId: tracked.paneId,
+                seenInStatus: tracked.seenInStatus,
+              },
+            );
+            continue;
+          }
 
-        if (isIdle || missingTooLong || isTimedOut) {
           sessionsToClose.push({
             sessionId,
-            reason: isIdle ? 'idle' : isTimedOut ? 'timeout' : 'missing',
+            reason: isIdle ? 'idle' : 'missing',
           });
         }
       }
@@ -372,7 +388,16 @@ export class MultiplexerSessionManager {
       );
     }
 
-    return (await response.json()) as Record<string, { type: string }>;
+    const body = await response.text();
+    if (body.trim() === '') {
+      throw new Error('session status response was empty');
+    }
+
+    try {
+      return JSON.parse(body) as Record<string, { type: string }>;
+    } catch (err) {
+      throw new Error(`session status response was not valid JSON: ${err}`);
+    }
   }
 
   private async closeSession(
@@ -540,6 +565,10 @@ export class MultiplexerSessionManager {
     return event.properties?.info?.id ?? event.properties?.sessionID;
   }
 
+  private isRunningBackgroundJob(sessionId: string): boolean {
+    return this.backgroundJobBoard?.get(sessionId)?.state === 'running';
+  }
+
   async cleanup(): Promise<void> {
     this.stopPolling();