Browse Source

fix(task-session-manager): remove noisy hot-path debug logs, fix recovery log ordering, add transform recovery test

- Delete 'skipping non-synthetic part' log that fired on every user message
- Delete 'transform scanning part' log that fired on every orchestrator text part
- Move 'recovered stale orchestrator mapping' log after shouldManageSession re-check
- Add ponytail comment documenting the tool.execute.before agent-identity assumption
- Fix test name convention: executeTool.before → tool.execute.before
- Add test for messages.transform recovery path
Michael Henke 1 month ago
parent
commit
7945bc4c6c
2 changed files with 53 additions and 23 deletions
  1. 47 1
      src/hooks/task-session-manager/index.test.ts
  2. 6 22
      src/hooks/task-session-manager/index.ts

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

@@ -2048,7 +2048,7 @@ describe('task-session-manager hook', () => {
     expect(board.list('parent-1')).toHaveLength(0);
   });
 
-  test('recovers stale orchestrator mapping in executeTool.before', async () => {
+  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
 
@@ -2107,4 +2107,50 @@ describe('task-session-manager hook', () => {
       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',
+    );
+  });
 });

+ 6 - 22
src/hooks/task-session-manager/index.ts

@@ -197,15 +197,7 @@ export function createTaskSessionManagerHook(
       return undefined;
     }
 
-    if (part.synthetic !== true) {
-      log('[task-session-manager] skipping non-synthetic part', {
-        partType: part.type,
-        hasText: typeof part.text === 'string',
-        textPreview:
-          typeof part.text === 'string' ? part.text.slice(0, 80) : undefined,
-      });
-      return undefined;
-    }
+    if (part.synthetic !== true) return undefined;
 
     const status = parseTaskStatusOutput(part.text);
     if (!status) {
@@ -334,14 +326,15 @@ export function createTaskSessionManagerHook(
       if (toolName !== 'task') return;
       if (!input.sessionID) return;
       if (!options.shouldManageSession(input.sessionID)) {
-        // No agent-type guard here: at tool.execute.before time there's no
-        // message to inspect. The transform hook (messages.transform) has
-        // the message.info.agent guard instead.
+        // 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 (!options.shouldManageSession(input.sessionID)) return;
       }
       if (!isObjectRecord(output.args)) return;
 
@@ -571,15 +564,6 @@ export function createTaskSessionManagerHook(
         }
 
         for (const [partIndex, part] of message.parts.entries()) {
-          if (part.type === 'text' && typeof part.text === 'string') {
-            log('[task-session-manager] transform scanning part', {
-              messageIndex,
-              partIndex,
-              synthetic: part.synthetic,
-              hasTaskContent: /task_id:|<task[\s>]/i.test(part.text),
-              textPreview: part.text.slice(0, 100),
-            });
-          }
           updateFromInjectedCompletion(part, message, messageIndex, partIndex);
         }
       }