Merge pull request #840 from alvinunreal/omos/fix-836-bg-error-swallow

fix(task-session-manager): record background subagent session.error on board (issue #836)

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

@@ -1722,6 +1722,79 @@ describe('task-session-manager hook', () => {
     expect(job?.resultSummary).toBe('connection refused');
   });
 
+  test('child session.error (non-orchestrator) records failure on board', async () => {
+    const board = new BackgroundJobBoard();
+    // Child subagent sessions are not orchestrators, so shouldManageSession
+    // returns false for them. The error must still land on the board,
+    // otherwise idle reconciliation marks the job completed (false success).
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: () => false,
+    });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'designer',
+      description: 'design ui',
+    });
+    board.updateStatus({ taskID: 'child-1', state: 'running' });
+
+    await hook.event({
+      event: {
+        type: 'session.error',
+        properties: {
+          sessionID: 'child-1',
+          error: {
+            name: 'AI_APICallError',
+            message: 'Internal server error',
+          },
+        },
+      },
+    });
+
+    const job = board.get('child-1');
+    expect(job?.state).toBe('error');
+    expect(job?.resultSummary).toBe('Internal server error');
+  });
+
+  test('child session.error during fallback is not recorded on board', async () => {
+    const board = new BackgroundJobBoard();
+    // isFallbackInProgress is currently always-false for real children
+    // (they have no fallback chain), so this guard path is unreachable in
+    // production today. The test pins the defensive behavior for the day
+    // children gain a fallback chain.
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: () => false,
+      isFallbackInProgress: () => true,
+    });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'designer',
+      description: 'design ui',
+    });
+    board.updateStatus({ taskID: 'child-1', state: 'running' });
+
+    await hook.event({
+      event: {
+        type: 'session.error',
+        properties: {
+          sessionID: 'child-1',
+          error: {
+            name: 'AI_APICallError',
+            message: 'Internal server error',
+          },
+        },
+      },
+    });
+
+    const job = board.get('child-1');
+    expect(job?.state).toBe('running');
+  });
+
   test('completed reconciled job appears reusable and resumes via task', async () => {
     const board = new BackgroundJobBoard();
     const { hook } = createHook({ backgroundJobBoard: board });

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

@@ -1215,6 +1215,27 @@ export function createTaskSessionManagerHook(
               });
             }
           }
+        } else if (sessionId) {
+          // Child subagent sessions are not orchestrators, so the block
+          // above never runs for them. Without this, a failed background
+          // subagent leaves its job in `running` and the idle-reconciliation
+          // path (which has no shouldManageSession guard) marks it
+          // `completed` — a false success. A child with no fallback chain has
+          // nothing to retry into, so surface the failure on the board.
+          const props = input.event.properties as
+            | { error?: unknown }
+            | undefined;
+          if (options.isFallbackInProgress?.(sessionId)) return;
+          const job = backgroundJobBoard.get(sessionId);
+          if (job && job.state === 'running') {
+            backgroundJobBoard.updateStatus({
+              taskID: sessionId,
+              state: 'error',
+              resultSummary:
+                (props?.error as { message?: string } | undefined)?.message ??
+                'Session error',
+            });
+          }
         }
 
         return;

src/utils/env.test.ts

@@ -10,12 +10,17 @@ describe('isTruthyEnvValue', () => {
     expect(isTruthyEnvValue(value)).toBe(true);
   });
 
-  test.each([undefined, '', '0', 'false', 'no', 'off', 'anything'])(
-    '%p is not truthy',
-    (value) => {
-      expect(isTruthyEnvValue(value)).toBe(false);
-    },
-  );
+  test.each([
+    undefined,
+    '',
+    '0',
+    'false',
+    'no',
+    'off',
+    'anything',
+  ])('%p is not truthy', (value) => {
+    expect(isTruthyEnvValue(value)).toBe(false);
+  });
 });
 
 describe('isPluginDisabledByEnv', () => {