Browse Source

Merge pull request #701 from mhenke/fix/notification-diagnostics

fix(task-session-manager): recover stale orchestrator mapping at both gates
Mike Henke 1 month ago
parent
commit
1c9dfc2855

+ 2 - 0
docs/multiplexer-integration.md

@@ -225,6 +225,8 @@ For Herdr:
 | `even-vertical` | Opens new subagent panes down |
 | `tiled` | Opens new subagent panes to the right |
 
+> **Note:** `main_pane_size` is ignored by herdr. All layouts split from the parent pane.
+
 **Example: wide-screen layout**
 
 ```jsonc

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

@@ -17,6 +17,7 @@ function flushIdleReconcileDelay() {
 
 function createHook(options?: {
   shouldManageSession?: (sessionID: string) => boolean;
+  registerSessionAsOrchestrator?: (sessionID: string) => void;
   readContextMinLines?: number;
   readContextMaxFiles?: number;
   backgroundJobBoard?: BackgroundJobBoard;
@@ -40,6 +41,7 @@ function createHook(options?: {
       readContextMaxFiles: options?.readContextMaxFiles,
       backgroundJobBoard: options?.backgroundJobBoard,
       shouldManageSession: options?.shouldManageSession ?? (() => true),
+      registerSessionAsOrchestrator: options?.registerSessionAsOrchestrator,
       isFallbackInProgress: options?.isFallbackInProgress,
       coordinator: options?.coordinator,
     },
@@ -2261,4 +2263,108 @@ describe('task-session-manager hook', () => {
 
     expect(board.list('parent-1')).toHaveLength(0);
   });
+
+  test('recovers stale orchestrator mapping in tool.execute.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',
+    });
+  });
+
+  test('recovers stale orchestrator mapping in messages.transform', async () => {
+    const agentMap = new Map<string, string>();
+    agentMap.set('orchestrator-1', 'explorer'); // stale non-orchestrator value
+
+    const board = new BackgroundJobBoard();
+    board.registerLaunch({
+      taskID: 'child-transform-1',
+      parentSessionID: 'orchestrator-1',
+      agent: 'explorer',
+      description: 'transform recovery test',
+    });
+
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: (id) => agentMap.get(id) === 'orchestrator',
+      registerSessionAsOrchestrator: (id) => {
+        agentMap.set(id, 'orchestrator');
+      },
+    });
+
+    // Before recovery: stale mapping blocks transform processing
+    const messages = {
+      messages: [
+        {
+          info: {
+            role: 'user',
+            agent: 'orchestrator',
+            sessionID: 'orchestrator-1',
+          },
+          parts: [{ type: 'text', text: 'continue working' }],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, messages as never);
+
+    // After recovery: agentMap corrected, board reminders injected
+    expect(agentMap.get('orchestrator-1')).toBe('orchestrator');
+    expect(messages.messages[0].parts[0].text).toContain(
+      '### Background Job Board',
+    );
+    expect(messages.messages[0].parts[0].text).toContain('child-transform-1');
+  });
 });

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

@@ -99,6 +99,9 @@ export function createTaskSessionManagerHook(
     readContextMaxFiles?: number;
     backgroundJobBoard?: BackgroundJobStore;
     shouldManageSession: (sessionID: string) => boolean;
+    /** Register a session as orchestrator when the transform hook detects
+     *  an orchestrator message but the session isn't in the agent map yet. */
+    registerSessionAsOrchestrator?: (sessionID: string) => void;
     /** Optional guard: when provided, idle events for a session that is
      *  currently undergoing a foreground-fallback abort/re-prompt cycle
      *  will NOT trigger idle reconciliation. prevents marking a still-
@@ -210,7 +213,12 @@ export function createTaskSessionManagerHook(
     if (part.synthetic !== true) return undefined;
 
     const status = parseTaskStatusOutput(part.text);
-    if (!status) return undefined;
+    if (!status) {
+      log('[task-session-manager] synthetic part missing task status', {
+        textPreview: part.text.slice(0, 120),
+      });
+      return undefined;
+    }
     if (status.state !== 'completed' && status.state !== 'error') {
       return undefined;
     }
@@ -329,8 +337,17 @@ 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)) {
+        // ponytail: no agent-identity guard here — at tool.execute.before
+        // time there's no message to inspect. Only orchestrators call `task`
+        // in standard architecture; non-orchestrator false-positives are
+        // accepted because leaf agents don't use this tool.
+        options.registerSessionAsOrchestrator?.(input.sessionID);
+        if (!options.shouldManageSession(input.sessionID)) return;
+        log('[task-session-manager] recovered stale orchestrator mapping', {
+          sessionID: input.sessionID,
+        });
       }
       if (!isObjectRecord(output.args)) return;
 
@@ -361,6 +378,17 @@ export function createTaskSessionManagerHook(
         label,
       };
       pendingCallTracker.add(pendingCall);
+      log(
+        '[task-session-manager] tool.execute.before task — pending call created',
+        {
+          callId: pendingCall.callId,
+          parentSessionId: pendingCall.parentSessionId,
+          agentType: pendingCall.agentType,
+          label: pendingCall.label,
+          inputCallID: input.callID,
+          inputSessionID: input.sessionID,
+        },
+      );
 
       if (typeof args.task_id !== 'string' || args.task_id.trim() === '') {
         return;
@@ -427,6 +455,16 @@ export function createTaskSessionManagerHook(
       if (input.tool.toLowerCase() !== 'task') return;
 
       const pending = pendingCallTracker.take(input.callID, input.sessionID);
+      log('[task-session-manager] tool.execute.after task', {
+        callID: input.callID,
+        sessionID: input.sessionID,
+        hasPending: !!pending,
+        outputType: typeof output.output,
+        outputPreview:
+          typeof output.output === 'string'
+            ? output.output.slice(0, 120)
+            : undefined,
+      });
 
       if (!pending || typeof output.output !== 'string') return;
       const launch = parseTaskLaunchOutput(output.output);
@@ -530,7 +568,12 @@ export function createTaskSessionManagerHook(
           !message.info.sessionID ||
           !options.shouldManageSession(message.info.sessionID)
         ) {
-          continue;
+          const sessionID = message.info.sessionID;
+          if (!sessionID || message.info.agent !== 'orchestrator') {
+            continue;
+          }
+          options.registerSessionAsOrchestrator?.(sessionID);
+          if (!options.shouldManageSession(sessionID)) continue;
         }
 
         for (const [partIndex, part] of message.parts.entries()) {

+ 3 - 0
src/index.ts

@@ -313,6 +313,9 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
       backgroundJobBoard: backgroundJobCoordinator,
       shouldManageSession: (sessionID) =>
         sessionAgentMap.get(sessionID) === 'orchestrator',
+      registerSessionAsOrchestrator: (sessionID) => {
+        sessionAgentMap.set(sessionID, 'orchestrator');
+      },
       isFallbackInProgress: (sessionID) =>
         foregroundFallback.isFallbackInProgress(sessionID),
       coordinator: sessionLifecycle,