Prechádzať zdrojové kódy

fix(task-session-manager): never apply call-specific metadata on unresolved-identity takes

GoldJohnKing 4 dní pred
rodič
commit
3011fcaa72

Rozdielové dáta súboru neboli zobrazené, pretože súbor je príliš veľký
+ 0 - 1
src/hooks/task-session-manager/codemap.md


+ 98 - 6
src/hooks/task-session-manager/parallel-same-agent-pairing.test.ts

@@ -334,27 +334,119 @@ describe('parallel same-agent pairing (incident 2026-09-12)', () => {
     // Ticket A was bound to sA and released on the terminal status —
     // only B's admission slot remains held.
     expect(concurrency.snapshot()).toEqual({ active: 1, queued: 0 });
-    // The board record carries the authoritative task ID from A's own
-    // output, with A's label (placeholder corrected).
-    expect(board.get(sA)?.description).toBe(L_A);
+    // The board record keeps everything output-authoritative (task ID,
+    // state, result text from A's own output), but the drained pending
+    // was consumed without verified identity — its label/objective may
+    // belong to sibling B — so the metadata floor applies: the record
+    // keeps the honest placeholder instead of a possibly-wrong label.
+    expect(board.get(sA)?.description).toBe('unattributed oracle task');
     expect(board.get(sA)?.state).toBe('completed');
+    expect(board.get(sA)?.resultSummary).toBe('Review finished.');
 
     // after B resolves through the normal sole-survivor take — the
-    // burst did not strand anything or poison the parent.
+    // burst did not strand anything or poison the parent. The take is
+    // still flagged: A's unresolved drain shifted the sole-survivor
+    // window, so B's label cannot be trusted either.
     await hook['tool.execute.after'](
       { tool: 'task', sessionID: PARENT },
       { output: completed(sB) },
     );
-    expect(board.get(sB)?.description).toBe(L_B);
+    expect(board.get(sB)?.description).toBe('unattributed oracle task');
+    expect(board.get(sB)?.state).toBe('completed');
+    expect(board.get(sB)?.resultSummary).toBe('Review finished.');
     expect(concurrency.snapshot()).toEqual({ active: 0, queued: 0 });
 
     // A subsequent no-ID call for this parent still works end-to-end.
+    // The parent's unresolved window persists for the session, so the
+    // sole take is flagged as well; with no placeholder record for sC
+    // the fresh registration falls back to registerLaunch's generic
+    // default label.
     await hook['tool.execute.before'](...noIDBefore(L_C));
     await hook['tool.execute.after'](
       { tool: 'task', sessionID: PARENT },
       { output: HOST_LAUNCH(sC) },
     );
-    expect(board.get(sC)?.description).toBe(L_C);
+    expect(board.get(sC)?.description).toBe('background oracle task');
+  });
+
+  test('eviction variant: pre-consumed pending degrades the burst without corrupting it (B)', async () => {
+    const board = new BackgroundJobBoard();
+    const tracker = createPendingCallTracker();
+    const hook = createHook(board, { pendingCallTracker: tracker });
+    const sA = 'ses_aaaa1111';
+    const sB = 'ses_bbbb2222';
+    const sC = 'ses_dddd4444';
+    const completed = (taskID: string) =>
+      [
+        `task_id: ${taskID}`,
+        'state: completed',
+        '',
+        '<task_result>',
+        'Review finished.',
+        '</task_result>',
+      ].join('\n');
+    const noIDBefore = (description: string) =>
+      [
+        { tool: 'task', sessionID: PARENT },
+        {
+          args: {
+            subagent_type: 'oracle',
+            description,
+            prompt: 'do the review',
+            background: true,
+          },
+        },
+      ] as const;
+
+    // Burst of three no-ID, no-title calls; the pending cap evicts the
+    // oldest pending (its ticket was released at eviction —
+    // pre-existing behavior) before any after-hook fires.
+    await hook['tool.execute.before'](...noIDBefore(L_A));
+    await hook['tool.execute.before'](...noIDBefore(L_B));
+    await hook['tool.execute.before'](...noIDBefore(L_C));
+    tracker.take('parent-1:anonymous-1');
+
+    // No-title children are ambiguous → placeholders claim no pending.
+    await hook.event(created({ child: sA }));
+    await hook.event(created({ child: sB }));
+    await hook.event(created({ child: sC }));
+    expect(board.get(sA)?.description).toBe('unattributed oracle task');
+    expect(board.get(sB)?.description).toBe('unattributed oracle task');
+    expect(board.get(sC)?.description).toBe('unattributed oracle task');
+
+    // The evicted call's late after-hook: two pendings remain, so
+    // take() refuses and the drain fallback consumes B's pending —
+    // flagged unresolved, so sA never receives a sibling label.
+    await hook['tool.execute.after'](
+      { tool: 'task', sessionID: PARENT },
+      { output: completed(sA) },
+    );
+    expect(board.get(sA)?.description).toBe('unattributed oracle task');
+    expect(board.get(sA)?.state).toBe('completed');
+    expect(board.get(sA)?.resultSummary).toBe('Review finished.');
+
+    // The sibling's after steals the shifted window (sole survivor C,
+    // armed by the drain) — flagged too, so sB also stays generic.
+    await hook['tool.execute.after'](
+      { tool: 'task', sessionID: PARENT },
+      { output: completed(sB) },
+    );
+    expect(board.get(sB)?.description).toBe('unattributed oracle task');
+    expect(board.get(sB)?.state).toBe('completed');
+    expect(board.get(sB)?.resultSummary).toBe('Review finished.');
+
+    // C's own after arrives last: no pending remains (its pending was
+    // consumed by B's window-shifted take), so its output drains
+    // nothing and drops. The cascade terminates degraded — sC keeps
+    // its honest placeholder instead of a stolen or poisoned record —
+    // and nothing is stranded.
+    await hook['tool.execute.after'](
+      { tool: 'task', sessionID: PARENT },
+      { output: completed(sC) },
+    );
+    expect(board.get(sC)?.description).toBe('unattributed oracle task');
+    expect(board.get(sC)?.state).toBe('running');
+    expect(tracker.peekByParent(PARENT)).toBeUndefined();
   });
 
   test('B1 drain fallback logs exactly one deterministic warning per burst', async () => {

+ 66 - 0
src/hooks/task-session-manager/pending-call-tracker.test.ts

@@ -305,4 +305,70 @@ describe('takeUnresolvedFirstMatch', () => {
       tracker.takeUnresolvedFirstMatch('parent-1', { identityTaskID: 'ses_x' }),
     ).toBeUndefined();
   });
+
+  test('flags the drained call unresolved and arms the parent window', () => {
+    const tracker = createPendingCallTracker();
+    tracker.add(pending({ callId: 'a' }));
+    tracker.add(pending({ callId: 'b' }));
+
+    const taken = tracker.takeUnresolvedFirstMatch('parent-1', {
+      identityTaskID: 'ses_x',
+    });
+
+    expect(taken?.callId).toBe('a');
+    expect(taken?.identityUnresolved).toBe(true);
+
+    // Window-shift propagation: the later no-callId sole take for the
+    // same parent is flagged too — "sole survivor" no longer proves
+    // identity once an unresolved drain shifted the ordering argument.
+    const sole = tracker.take(undefined, 'parent-1');
+    expect(sole?.callId).toBe('b');
+    expect(sole?.identityUnresolved).toBe(true);
+  });
+
+  test('armed window never flags a claim-verified takeByTaskID take', () => {
+    const tracker = createPendingCallTracker();
+    tracker.add(pending({ callId: 'a', earlyRegisteredTaskID: 'ses_claimed' }));
+    tracker.add(pending({ callId: 'b' }));
+
+    // Drain the unmarked pending (a is fenced by its early-registration
+    // claim), arming the parent's unresolved window.
+    expect(
+      tracker.takeUnresolvedFirstMatch('parent-1', {
+        identityTaskID: 'ses_x',
+      })?.callId,
+    ).toBe('b');
+
+    // A takeByTaskID take is identity-verified by the early
+    // registration's claim: no unresolved flag.
+    const claimed = tracker.takeByTaskID('parent-1', 'ses_claimed');
+    expect(claimed?.callId).toBe('a');
+    expect(claimed?.identityUnresolved).toBeUndefined();
+  });
+
+  test('clearSession resets the unresolved window for that parent', () => {
+    const tracker = createPendingCallTracker();
+    tracker.add(pending({ callId: 'a' }));
+    tracker.takeUnresolvedFirstMatch('parent-1', { identityTaskID: 'ses_x' });
+
+    tracker.clearSession('parent-1');
+
+    // A fresh pending in the cleared window resolves normally.
+    tracker.add(pending({ callId: 'b' }));
+    const sole = tracker.take(undefined, 'parent-1');
+    expect(sole?.callId).toBe('b');
+    expect(sole?.identityUnresolved).toBeUndefined();
+  });
+
+  test('clearAll resets every unresolved window', () => {
+    const tracker = createPendingCallTracker();
+    tracker.add(pending({ callId: 'a', parentSessionId: 'parent-1' }));
+    tracker.takeUnresolvedFirstMatch('parent-1', { identityTaskID: 'ses_x' });
+
+    tracker.clearAll();
+
+    tracker.add(pending({ callId: 'b', parentSessionId: 'parent-1' }));
+    const sole = tracker.take(undefined, 'parent-1');
+    expect(sole?.identityUnresolved).toBeUndefined();
+  });
 });

+ 28 - 2
src/hooks/task-session-manager/pending-call-tracker.ts

@@ -29,6 +29,10 @@ export interface PendingTaskCall {
   earlyRegisteredTaskID?: string;
   earlyRegistration?: EarlyTaskRegistration;
   earlyRegistrationRejected?: boolean;
+  /** Consumed without verified call identity (no-ID drain fallback or a
+   *  window-shifted sole take): the label/objective may belong to a
+   *  sibling call and must not be painted onto the board record. */
+  identityUnresolved?: boolean;
 }
 
 const MAX_PENDING_TASK_CALLS = 100;
@@ -59,8 +63,10 @@ export interface PendingCallTracker {
    *  `identityTaskID` (that pending provably belongs to another call).
    *  Pendings claimed by an early registration or fenced for another
    *  board generation are left for their owners. Consumption is
-   *  recorded exactly like `take()`. Returns undefined when no
-   *  eligible pending exists. */
+   *  recorded exactly like `take()`. The consumed call is marked
+   *  `identityUnresolved` and arms the parent's unresolved window, so
+   *  later no-callID sole-survivor takes for the same parent are
+   *  flagged too. Returns undefined when no eligible pending exists. */
   takeUnresolvedFirstMatch(
     parentSessionId: string,
     selection?: {
@@ -99,6 +105,12 @@ export function createPendingCallTracker(
   const pendingCalls = new Map<string, PendingTaskCall>();
   let anonymousPendingCallId = 0;
 
+  /** Parents where a pending was consumed through the unresolved-identity
+   *  drain fallback. The sole-survivor argument for a later no-callID
+   *  take only holds while every prior take was resolved; one unresolved
+   *  drain shifts the window, so subsequent sole takes are flagged too. */
+  const unresolvedDrainParents = new Set<string>();
+
   /** Calls already consumed by their tool.execute.after, kept briefly so
    * late no-title session.created events can be recognized as possibly
    * stale children of a consumed call instead of claiming an unrelated
@@ -189,6 +201,13 @@ export function createPendingCallTracker(
         const sole = solePendingIdForParent(parentSessionId);
         if (!sole) return undefined;
         callId = sole;
+        // Window-shift propagation: an unresolved drain earlier in this
+        // parent's burst means "sole survivor belongs to this call" no
+        // longer proves identity — flag the taken pending unresolved.
+        if (unresolvedDrainParents.has(parentSessionId)) {
+          const solePending = pendingCalls.get(sole);
+          if (solePending) solePending.identityUnresolved = true;
+        }
       }
       if (!callId) return undefined;
       const pending = pendingCalls.get(callId);
@@ -353,6 +372,11 @@ export function createPendingCallTracker(
         }
         pendingCalls.delete(callId);
         recordConsumed(call);
+        // Identity was not verified: the consumed pending's metadata may
+        // belong to a sibling call, and the parent's sole-survivor
+        // window has shifted for any later no-callID take.
+        call.identityUnresolved = true;
+        unresolvedDrainParents.add(parentSessionId);
         return call;
       }
       return undefined;
@@ -417,6 +441,7 @@ export function createPendingCallTracker(
           consumedCalls.delete(callId);
         }
       }
+      unresolvedDrainParents.delete(sessionId);
       // Release queued tickets before active tickets. Releasing an active
       // ticket pumps the scheduler, so doing it in insertion order could
       // admit a later call just as the parent is being deleted.
@@ -429,6 +454,7 @@ export function createPendingCallTracker(
       const removed = [...pendingCalls.values()].reverse();
       pendingCalls.clear();
       consumedCalls.clear();
+      unresolvedDrainParents.clear();
       for (const pending of removed) releaseCallLease(pending);
     },
 

+ 18 - 2
src/hooks/task-session-manager/tool-execute-hooks.ts

@@ -584,13 +584,29 @@ function registerTaskOutputLaunch(
     );
   }
 
+  if (pending.identityUnresolved) {
+    log(
+      '[task-session-manager] registered authoritative task ID with generic metadata (identity unresolved)',
+      { taskID, callID: pending.callId },
+    );
+  }
+
   try {
     return deps.backgroundJobBoard.registerLaunch({
       taskID,
       parentSessionID: pending.parentSessionId,
       agent: pending.agentType,
-      description: pending.label,
-      objective: pending.fullObjective ?? pending.label,
+      // Identity was unresolved (no-ID drain or window-shifted take):
+      // the label/objective may belong to a sibling call, so never
+      // paint them. Existing placeholder records keep their honest
+      // description; fresh records fall back to registerLaunch's
+      // generic default.
+      ...(pending.identityUnresolved
+        ? {}
+        : {
+            description: pending.label,
+            objective: pending.fullObjective ?? pending.label,
+          }),
       background: exactCallConfirmed && pending.background,
       preserveRun:
         pending.earlyRegisteredTaskID === taskID ||

Niektoré súbory nie sú zobrazené, pretože je v týchto rozdielových dátach zmenené mnoho súborov