Browse Source

fix(task-session-manager): recover stale orchestrator mapping at both gates

Background task completions were silently dropped because two critical paths
both gated on shouldManageSession before registering the orchestrator session:

- executeTool.before: stale agentMap value blocked pending call creation,
  so executeTool.after could never register the board entry
- transform hook: stale agentMap value blocked completion injection scanning

Both paths now call registerSessionAsOrchestrator (unconditional set) before
giving up, matching the recovery pattern Greptile suggested in PR #696.

Closes #695
Michael Henke 1 month ago
parent
commit
8f4d5823ec

+ 63 - 0
src/hooks/task-session-manager/index.test.ts

@@ -5,6 +5,7 @@ import { createTaskSessionManagerHook } from './index';
 
 function createHook(options?: {
   shouldManageSession?: (sessionID: string) => boolean;
+  registerSessionAsOrchestrator?: (sessionID: string) => void;
   readContextMinLines?: number;
   readContextMaxFiles?: number;
   backgroundJobBoard?: BackgroundJobBoard;
@@ -28,6 +29,8 @@ function createHook(options?: {
       readContextMaxFiles: options?.readContextMaxFiles,
       backgroundJobBoard: options?.backgroundJobBoard,
       shouldManageSession: options?.shouldManageSession ?? (() => true),
+      registerSessionAsOrchestrator:
+        options?.registerSessionAsOrchestrator,
       isFallbackInProgress: options?.isFallbackInProgress,
       coordinator: options?.coordinator,
     },
@@ -2045,4 +2048,64 @@ describe('task-session-manager hook', () => {
 
     expect(board.list('parent-1')).toHaveLength(0);
   });
+
+  test('recovers stale orchestrator mapping in executeTool.before', async () => {
+    const agentMap = new Map<string, string>();
+    agentMap.set('orchestrator-1', 'explorer'); // stale non-orchestrator value
+
+    const board = new BackgroundJobBoard();
+
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: (id) => agentMap.get(id) === 'orchestrator',
+      registerSessionAsOrchestrator: (id) => {
+        agentMap.set(id, 'orchestrator');
+      },
+    });
+
+    // Before recovery: stale mapping blocks pending call creation
+    await hook['tool.execute.before'](
+      {
+        tool: 'task',
+        sessionID: 'orchestrator-1',
+        callID: 'call-recovery',
+      },
+      {
+        args: {
+          subagent_type: 'explorer',
+          description: 'test recovery',
+        },
+      },
+    );
+
+    // After recovery: agentMap now has 'orchestrator' for this session
+    expect(agentMap.get('orchestrator-1')).toBe('orchestrator');
+
+    // executeTool.after finds the pending call and registers the board entry
+    await hook['tool.execute.after'](
+      {
+        tool: 'task',
+        sessionID: 'orchestrator-1',
+        callID: 'call-recovery',
+      },
+      {
+        output: [
+          'task_id: child-recovery-1',
+          'state: running',
+          '',
+          '<task_result>',
+          'Background task started.',
+          '</task_result>',
+        ].join('\n'),
+      },
+    );
+
+    const jobs = board.list('orchestrator-1');
+    expect(jobs).toHaveLength(1);
+    expect(jobs[0]).toMatchObject({
+      taskID: 'child-recovery-1',
+      parentSessionID: 'orchestrator-1',
+      state: 'running',
+    });
+  });
 });

+ 4 - 2
src/hooks/task-session-manager/index.ts

@@ -332,8 +332,10 @@ export function createTaskSessionManagerHook(
     ): Promise<void> => {
       const toolName = input.tool.toLowerCase();
       if (toolName !== 'task') return;
-      if (!input.sessionID || !options.shouldManageSession(input.sessionID)) {
-        return;
+      if (!input.sessionID) return;
+      if (!options.shouldManageSession(input.sessionID)) {
+        options.registerSessionAsOrchestrator?.(input.sessionID);
+        if (!options.shouldManageSession(input.sessionID)) return;
       }
       if (!isObjectRecord(output.args)) return;
 

+ 1 - 3
src/index.ts

@@ -314,9 +314,7 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       shouldManageSession: (sessionID) =>
         sessionAgentMap.get(sessionID) === 'orchestrator',
       registerSessionAsOrchestrator: (sessionID) => {
-        if (!sessionAgentMap.has(sessionID)) {
-          sessionAgentMap.set(sessionID, 'orchestrator');
-        }
+        sessionAgentMap.set(sessionID, 'orchestrator');
       },
       isFallbackInProgress: (sessionID) =>
         foregroundFallback.isFallbackInProgress(sessionID),

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

@@ -125,7 +125,12 @@ export class TmuxMultiplexer implements Multiplexer {
     }
 
     try {
-      // Send Ctrl+C for graceful shutdown
+      // Graceful shutdown sequence:
+      // 1. Send Ctrl+C to the pane to trigger graceful termination of child processes
+      // 2. Wait 250ms to allow processes time to handle SIGINT and clean up
+      // 3. Fallback to kill-pane if graceful termination fails or times out
+      // This ensures child processes (e.g., opencode attach sessions) can exit cleanly
+      // before we forcefully terminate the tmux pane.
       log('[tmux] closePane: sending Ctrl+C', { paneId });
       const ctrlCProc = crossSpawn([tmux, 'send-keys', '-t', paneId, 'C-c'], {
         stdout: 'pipe',