Explorar el Código

fix(task-session-manager): preserve serialized NamedError detail in board error summaries

The core publishes session errors through NamedError.toObject(), whose
wire shape is { name, data } with the human-readable message in
data.message (APIError, ProviderAuthError, ...). The session.error
handler in the event router read only the top-level message field, which
is undefined for every serialized NamedError, so the board recorded the
generic 'Session error' even when the detail existed two levels down.

Extract data.message first, then fall back to a top-level message for
non-NamedError payloads, in both the managed-session and child branches.
Terminalization, fallback, generation, and reconciliation decisions are
unchanged.

Diagnostics motivation: #1200.
dhaern hace 1 día
padre
commit
201c798c55

+ 30 - 4
src/hooks/task-session-manager/event-router.ts

@@ -25,6 +25,34 @@ import type { RevivedRunTracker } from './revived-run-tracker';
 
 type BackgroundJobRecord = NonNullable<ReturnType<BackgroundJobStore['get']>>;
 
+/**
+ * Extract a human-readable message from a serialized session error.
+ *
+ * The core publishes session errors through NamedError.toObject(), whose
+ * wire shape is `{ name: string; data: ... }` — the message lives in
+ * `data.message` (APIError, ProviderAuthError, ...), not at the top
+ * level. Reading only `error.message` yields undefined for every
+ * serialized NamedError and the board fell back to the generic
+ * "Session error" even when the detail existed two levels down (#1200
+ * diagnostics). Plain `{ message }` shapes are still honored for
+ * non-NamedError payloads.
+ */
+function structuredErrorMessage(error: unknown): string | undefined {
+  if (!isRecordLike(error)) return undefined;
+  const data = error.data;
+  if (isRecordLike(data)) {
+    const inner = data.message;
+    if (typeof inner === 'string' && inner.length > 0) return inner;
+  }
+  const direct = error.message;
+  if (typeof direct === 'string' && direct.length > 0) return direct;
+  return undefined;
+}
+
+function isRecordLike(value: unknown): value is Record<string, unknown> {
+  return typeof value === 'object' && value !== null;
+}
+
 interface SessionEventGenerationFence {
   generation: number;
   /** A new generation must see a live activity fence before lifecycle events. */
@@ -605,8 +633,7 @@ export async function handleEvent(
             state: 'error',
             expectedGeneration: observation?.generation,
             resultSummary:
-              (props?.error as { message?: string } | undefined)?.message ??
-              'Session error',
+              structuredErrorMessage(props?.error) ?? 'Session error',
           });
           if (updated) deps.revivedRunTracker?.onTerminal(updated);
         }
@@ -639,8 +666,7 @@ export async function handleEvent(
           state: 'error',
           expectedGeneration: observation?.generation,
           resultSummary:
-            (props?.error as { message?: string } | undefined)?.message ??
-            'Session error',
+            structuredErrorMessage(props?.error) ?? 'Session error',
         });
         if (updated) deps.revivedRunTracker?.onTerminal(updated);
       }

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

@@ -4658,6 +4658,175 @@ describe('task-session-manager hook', () => {
     expect(job?.resultSummary).toBe('Internal server error');
   });
 
+  test('child session.error preserves serialized NamedError detail (data.message)', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: () => false,
+    });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'oracle',
+      description: 'audit the diff',
+    });
+    board.updateStatus({ taskID: 'child-1', state: 'running' });
+
+    // The core publishes session errors via NamedError.toObject():
+    // `{ name, data }` with the message inside data (APIError,
+    // ProviderAuthError, ...). A top-level-only read loses it (#1200).
+    await hook.event({
+      event: {
+        type: 'session.error',
+        properties: {
+          sessionID: 'child-1',
+          error: {
+            name: 'APIError',
+            data: {
+              message: 'stream stall timeout',
+              isRetryable: true,
+            },
+          },
+        },
+      },
+    });
+
+    const job = board.get('child-1');
+    expect(job?.state).toBe('error');
+    expect(job?.resultSummary).toBe('stream stall timeout');
+  });
+
+  test('child session.error without any message falls back to generic summary', async () => {
+    const board = new BackgroundJobBoard();
+    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: 'APIError', data: { isRetryable: false } },
+        },
+      },
+    });
+
+    const job = board.get('child-1');
+    expect(job?.state).toBe('error');
+    expect(job?.resultSummary).toBe('Session error');
+  });
+
+  test('child session.error prefers data.message over top-level message', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      shouldManageSession: () => false,
+    });
+
+    board.registerLaunch({
+      taskID: 'child-1',
+      parentSessionID: 'parent-1',
+      agent: 'oracle',
+      description: 'audit the diff',
+    });
+    board.updateStatus({ taskID: 'child-1', state: 'running' });
+
+    await hook.event({
+      event: {
+        type: 'session.error',
+        properties: {
+          sessionID: 'child-1',
+          error: {
+            name: 'APIError',
+            message: 'Instance name (generic)',
+            data: { message: 'stream stall timeout', isRetryable: true },
+          },
+        },
+      },
+    });
+
+    // NamedError instances carry their class name as the top-level
+    // message; the human-readable detail is data.message.
+    expect(board.get('child-1')?.resultSummary).toBe('stream stall timeout');
+  });
+
+  test('child session.error with empty data.message falls back to top-level message', async () => {
+    const board = new BackgroundJobBoard();
+    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',
+            data: { message: '' },
+          },
+        },
+      },
+    });
+
+    expect(board.get('child-1')?.resultSummary).toBe('Internal server error');
+  });
+
+  test('managed session.error preserves serialized NamedError detail (data.message)', async () => {
+    const board = new BackgroundJobBoard();
+    const { hook } = createHook({
+      backgroundJobBoard: board,
+      // No chain / chain exhausted / fallback disabled → error is final.
+      willAttemptFallback: () => false,
+    });
+
+    board.registerLaunch({
+      taskID: 'parent-1',
+      parentSessionID: 'root-1',
+      agent: 'orchestrator',
+      description: 'background session',
+    });
+    board.updateStatus({ taskID: 'parent-1', state: 'running' });
+
+    await hook.event({
+      event: {
+        type: 'session.error',
+        properties: {
+          sessionID: 'parent-1',
+          error: {
+            name: 'ProviderAuthError',
+            data: { providerID: 'acme', message: 'Invalid API key' },
+          },
+        },
+      },
+    });
+
+    const job = board.get('parent-1');
+    expect(job?.state).toBe('error');
+    expect(job?.resultSummary).toBe('Invalid API key');
+  });
+
   test('child session.error during fallback is not recorded on board', async () => {
     const board = new BackgroundJobBoard();
     // isFallbackInProgress is currently always-false for real children