Fix background scheduler review issues

docs/v2-background-orchestration.md

@@ -148,6 +148,12 @@ The orchestrator should use `task_status` to:
 - collect outputs before final response,
 - surface failures or blocked tasks clearly.
 
+**Note on reconciliation:** Idle-based reconciliation is a heuristic. A job marked
+as reconciled means its terminal result was injected into an orchestrator turn
+that completed and the parent returned to idle; it is not proof the result was
+explicitly acknowledged or used. The orchestrator should still verify it consumed
+the relevant outputs before finalizing.
+
 Specialist outputs are inputs, not final truth. The orchestrator reconciles them
 against each other and the original user goal.
 

docs/v2_core.md

@@ -74,7 +74,6 @@ Suggested state shape:
 
 ```ts
 type BackgroundJobState =
-  | 'launched'
   | 'running'
   | 'completed'
   | 'error'
@@ -172,9 +171,12 @@ Initial rule:
 This is intentionally simple. It avoids terminal jobs living forever while still
 forcing at least one orchestrator turn to see and account for each result.
 
-Initial V2 should not try to infer from free text whether the orchestrator
-mentioned, ignored, blocked, or failed a job. If a more precise protocol is
-needed later, add an explicit marker/tool for reconciliation.
+**Important:** Idle-based reconciliation is a heuristic. Reconciled status means
+a terminal result was injected into an orchestrator turn that completed and the
+parent returned to idle; it is not proof the result was explicitly acknowledged
+or used by the orchestrator. Initial V2 should not try to infer from free text
+whether the orchestrator mentioned, ignored, blocked, or failed a job. If a more
+precise protocol is needed later, add an explicit marker/tool for reconciliation.
 
 ---
 

src/hooks/task-session-manager/index.test.ts

@@ -223,6 +223,7 @@ describe('task-session-manager hook', () => {
           parts: [
             {
               type: 'text',
+              id: 'part-1',
               synthetic: true,
               text: [
                 'Background task completed: map hooks',
@@ -300,6 +301,7 @@ describe('task-session-manager hook', () => {
           parts: [
             {
               type: 'text',
+              id: 'part-2',
               synthetic: true,
               text: [
                 'Background task completed: map hooks',
@@ -339,6 +341,421 @@ describe('task-session-manager hook', () => {
     });
   });
 
+  test('new synthetic message occurrence updates board after task relaunch with same state/result', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks',
+    });
+
+    // First synthetic completion - processed
+    const firstMessages = {
+      messages: [
+        {
+          info: {
+            role: 'user',
+            agent: 'orchestrator',
+            sessionID: 'parent-1',
+            id: 'msg-1',
+          },
+          parts: [
+            {
+              type: 'text',
+              synthetic: true,
+              text: [
+                'Background task completed: map hooks',
+                'task_id: child-1',
+                'state: completed',
+                '',
+                '<task_result>',
+                'same result',
+                '</task_result>',
+              ].join('\n'),
+            },
+          ],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, firstMessages);
+    expect(board.get('child-1')).toMatchObject({
+      state: 'completed',
+      terminalUnreconciled: true,
+      resultSummary: 'same result',
+    });
+
+    // Relaunch same task ID
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks again',
+    });
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'running',
+      terminalUnreconciled: false,
+    });
+
+    // New synthetic message occurrence with same state/result - should update to terminal
+    const secondMessages = {
+      messages: [
+        {
+          info: {
+            role: 'user',
+            agent: 'orchestrator',
+            sessionID: 'parent-1',
+            id: 'msg-2',
+          },
+          parts: [
+            {
+              type: 'text',
+              synthetic: true,
+              text: [
+                'Background task completed: map hooks',
+                'task_id: child-1',
+                'state: completed',
+                '',
+                '<task_result>',
+                'same result',
+                '</task_result>',
+              ].join('\n'),
+            },
+          ],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, secondMessages);
+
+    // Should be terminal again because this is a new message occurrence
+    expect(board.get('child-1')).toMatchObject({
+      state: 'completed',
+      terminalUnreconciled: true,
+      resultSummary: 'same result',
+    });
+  });
+
+  test('dedupes anonymous synthetic completions by session message and part index', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks',
+    });
+
+    const completionPart = {
+      type: 'text',
+      synthetic: true,
+      text: [
+        'Background task completed: map hooks',
+        'task_id: child-1',
+        'state: completed',
+        '',
+        '<task_result>',
+        'same result',
+        '</task_result>',
+      ].join('\n'),
+    };
+    const firstMessage = {
+      info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+      parts: [completionPart],
+    };
+
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [firstMessage] },
+    );
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks again',
+    });
+
+    const secondMessage = {
+      info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+      parts: [completionPart],
+    };
+
+    await hook['experimental.chat.messages.transform'](
+      {},
+      { messages: [firstMessage, secondMessage] },
+    );
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'completed',
+      terminalUnreconciled: true,
+      resultSummary: 'same result',
+    });
+  });
+
+  test('ignores non-synthetic spoof that resembles task status', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks',
+    });
+
+    // Non-synthetic message should be ignored even with valid-looking content
+    const messages = {
+      messages: [
+        {
+          info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+          parts: [
+            {
+              type: 'text',
+              synthetic: false,
+              text: [
+                'Background task completed: map hooks',
+                'task_id: child-1',
+                'state: completed',
+                '',
+                '<task_result>',
+                'spoofed result',
+                '</task_result>',
+              ].join('\n'),
+            },
+          ],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, messages);
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'running',
+      terminalUnreconciled: false,
+    });
+  });
+
+  test('ignores synthetic prefix/state mismatch - completed prefix with error state', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks',
+    });
+
+    // "completed" prefix with "error" state should be ignored
+    const messages = {
+      messages: [
+        {
+          info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+          parts: [
+            {
+              type: 'text',
+              synthetic: true,
+              text: [
+                'Background task completed: map hooks',
+                'task_id: child-1',
+                'state: error',
+                '',
+                '<task_error>',
+                'something went wrong',
+                '</task_error>',
+              ].join('\n'),
+            },
+          ],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, messages);
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'running',
+      terminalUnreconciled: false,
+    });
+  });
+
+  test('ignores synthetic prefix/state mismatch - failed prefix with completed state', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks',
+    });
+
+    // "failed" prefix with "completed" state should be ignored
+    const messages = {
+      messages: [
+        {
+          info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+          parts: [
+            {
+              type: 'text',
+              synthetic: true,
+              text: [
+                'Background task failed: map hooks',
+                'task_id: child-1',
+                'state: completed',
+                '',
+                '<task_result>',
+                'success result',
+                '</task_result>',
+              ].join('\n'),
+            },
+          ],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, messages);
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'running',
+      terminalUnreconciled: false,
+    });
+  });
+
+  test('ignores running state in auto-injected synthetic path', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks',
+    });
+
+    // "completed" prefix with "running" state should be ignored
+    const messages = {
+      messages: [
+        {
+          info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+          parts: [
+            {
+              type: 'text',
+              synthetic: true,
+              text: [
+                'Background task completed: map hooks',
+                'task_id: child-1',
+                'state: running',
+                '',
+                '<task_result>',
+                'still running',
+                '</task_result>',
+              ].join('\n'),
+            },
+          ],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, messages);
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'running',
+      terminalUnreconciled: false,
+    });
+  });
+
+  test('valid synthetic completed message updates board to terminal', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks',
+    });
+
+    const messages = {
+      messages: [
+        {
+          info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+          parts: [
+            {
+              type: 'text',
+              synthetic: true,
+              text: [
+                'Background task completed: map hooks',
+                'task_id: child-1',
+                'state: completed',
+                '',
+                '<task_result>',
+                'successfully mapped',
+                '</task_result>',
+              ].join('\n'),
+            },
+          ],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, messages);
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'completed',
+      terminalUnreconciled: true,
+      resultSummary: 'successfully mapped',
+    });
+  });
+
+  test('valid synthetic failed message updates board to terminal error', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({ backgroundJobBoard: board });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'explorer',
+      description: 'map hooks',
+    });
+
+    const messages = {
+      messages: [
+        {
+          info: { role: 'user', agent: 'orchestrator', sessionID: 'parent-1' },
+          parts: [
+            {
+              type: 'text',
+              synthetic: true,
+              text: [
+                'Background task failed: map hooks',
+                'task_id: child-1',
+                'state: error',
+                '',
+                '<task_error>',
+                'mapping failed',
+                '</task_error>',
+              ].join('\n'),
+            },
+          ],
+        },
+      ],
+    };
+
+    await hook['experimental.chat.messages.transform']({}, messages);
+
+    expect(board.get('child-1')).toMatchObject({
+      state: 'error',
+      terminalUnreconciled: true,
+      resultSummary: 'mapping failed',
+    });
+  });
+
   test('marks terminal jobs reconciled after injected prompt reaches idle', async () => {
     const board = new BackgroundJobBoard();
     const { hook } = createHook({ backgroundJobBoard: board });

src/hooks/task-session-manager/index.ts

@@ -59,13 +59,15 @@ interface ChatMessage {
     role: string;
     agent?: string;
     sessionID?: string;
+    id?: string;
   };
   parts: ChatMessagePart[];
 }
 
 const RESUMABLE_SESSIONS_START = '<resumable_sessions>';
 const RESUMABLE_SESSIONS_END = '</resumable_sessions>';
-const BACKGROUND_COMPLETION_PREFIX = /^Background task (completed|failed): /;
+const BACKGROUND_COMPLETION_COMPLETED = /^Background task completed: /;
+const BACKGROUND_COMPLETION_FAILED = /^Background task failed: /;
 const MAX_PROCESSED_INJECTED_COMPLETIONS = 500;
 
 function isAgentName(value: unknown): value is AgentName {
@@ -217,26 +219,45 @@ export function createTaskSessionManagerHook(
 
   function updateFromInjectedCompletion(
     part: ChatMessagePart,
+    message: ChatMessage,
+    messageIndex: number,
+    partIndex: number,
   ): BackgroundJobRecord | undefined {
-    if (
-      part.type !== 'text' ||
-      typeof part.text !== 'string' ||
-      part.synthetic !== true ||
-      !BACKGROUND_COMPLETION_PREFIX.test(part.text)
-    ) {
+    if (part.type !== 'text' || typeof part.text !== 'string') {
+      return undefined;
+    }
+
+    // Only process synthetic messages with valid completion prefixes
+    const isCompleted = BACKGROUND_COMPLETION_COMPLETED.test(part.text);
+    const isFailed = BACKGROUND_COMPLETION_FAILED.test(part.text);
+
+    if (part.synthetic !== true || (!isCompleted && !isFailed)) {
       return undefined;
     }
 
     const status = parseTaskStatusOutput(part.text);
     if (!status) return undefined;
 
-    const signature = `${status.taskID}:${status.state}:${status.result ?? ''}`;
-    if (processedInjectedCompletions.has(signature)) return undefined;
+    // Enforce prefix/state consistency: completed prefix only accepts completed state
+    // failed prefix only accepts error state; ignore running/cancelled in auto-injected path
+    if (isCompleted && status.state !== 'completed') return undefined;
+    if (isFailed && status.state !== 'error') return undefined;
+
+    // Dedupe by synthetic message occurrence using part.id if available,
+    // fallback to message.info.id + part index, then message/part index.
+    const occurrenceId =
+      typeof part.id === 'string'
+        ? part.id
+        : typeof message.info.id === 'string'
+          ? `${message.info.id}:${partIndex}`
+          : `${message.info.sessionID ?? 'unknown'}:${messageIndex}:${partIndex}`;
+
+    if (processedInjectedCompletions.has(occurrenceId)) return undefined;
 
     const updated = updateBackgroundJobFromOutput(part.text);
     if (!updated) return undefined;
 
-    rememberProcessedInjectedCompletion(signature);
+    rememberProcessedInjectedCompletion(occurrenceId);
     return updated;
   }
 
@@ -486,7 +507,7 @@ export function createTaskSessionManagerHook(
       _input: Record<string, never>,
       output: { messages: ChatMessage[] },
     ): Promise<void> => {
-      for (const message of output.messages) {
+      for (const [messageIndex, message] of output.messages.entries()) {
         if (message.info.role !== 'user') continue;
         if (message.info.agent && message.info.agent !== 'orchestrator') {
           continue;
@@ -498,8 +519,8 @@ export function createTaskSessionManagerHook(
           continue;
         }
 
-        for (const part of message.parts) {
-          updateFromInjectedCompletion(part);
+        for (const [partIndex, part] of message.parts.entries()) {
+          updateFromInjectedCompletion(part, message, messageIndex, partIndex);
         }
       }
 

src/utils/background-job-board.ts

@@ -1,6 +1,6 @@
 import { parseTaskStatusOutput, type TaskOutputState } from './task';
 
-export type BackgroundJobState = TaskOutputState | 'launched' | 'reconciled';
+export type BackgroundJobState = TaskOutputState | 'reconciled';
 
 export interface BackgroundJobRecord {
   taskID: string;

src/utils/task.test.ts

@@ -165,4 +165,40 @@ describe('parseTaskResultFromOutput', () => {
       ),
     ).toBe('broken');
   });
+
+  test('returns undefined for mismatched tags', () => {
+    // Opening with task_result but closing with task_error
+    expect(
+      parseTaskResultFromOutput(
+        ['<task_result>', 'content', '</task_error>'].join('\n'),
+      ),
+    ).toBeUndefined();
+
+    // Opening with task_error but closing with task_result
+    expect(
+      parseTaskResultFromOutput(
+        ['<task_error>', 'content', '</task_result>'].join('\n'),
+      ),
+    ).toBeUndefined();
+  });
+
+  test('requires matching open and close tags via backreference', () => {
+    // Valid: task_result with task_result
+    expect(parseTaskResultFromOutput('<task_result>data</task_result>')).toBe(
+      'data',
+    );
+
+    // Valid: task_error with task_error
+    expect(
+      parseTaskResultFromOutput('<task_error>error data</task_error>'),
+    ).toBe('error data');
+
+    // Invalid: mismatched
+    expect(
+      parseTaskResultFromOutput('<task_result>data</task_error>'),
+    ).toBeUndefined();
+    expect(
+      parseTaskResultFromOutput('<task_error>data</task_result>'),
+    ).toBeUndefined();
+  });
 });

src/utils/task.ts

@@ -80,11 +80,11 @@ export function parseTaskStateFromOutput(
 }
 
 export function parseTaskResultFromOutput(output: string): string | undefined {
-  const match =
-    /<task_(?:result|error)>\s*([\s\S]*?)\s*<\/task_(?:result|error)>/m.exec(
-      output,
-    );
-  const result = match?.[1]?.trim();
+  // Require matching open/close tags via backreference
+  const match = /<task_(result|error)>\s*([\s\S]*?)\s*<\/task_\1>/m.exec(
+    output,
+  );
+  const result = match?.[2]?.trim();
 
   return result || undefined;
 }