Browse Source

fix(task-session-manager): bound unresolved fallback handoff so empty transcripts can still stop

A transport failure on the retried promptAsync promoted the observation
handoff and left isObservationPending true with no later admit/reject.
The stop gate then deferred forever, so a valid empty transcript could
not confirm stopped and the board stayed running.

After promotion, keep fencing for one more expiry window, probe again,
then lift the fence without discarding the owner. A late result still
has a delivery owner; a still-empty transcript can confirm stopped.
dhaern 18 hours ago
parent
commit
e51e7ac3e2

+ 2 - 1
src/hooks/task-session-manager/fallback-observation-transfer.ts

@@ -28,7 +28,8 @@ import type { RevivedRunTracker } from './revived-run-tracker';
  * - `settleUnresolved` handles unknown outcomes (transport failed
  *   without a response — the host may still have accepted the replay):
  *   the prepared ownership CONVERTS into a tracked run instead of being
- *   dropped.
+ *   dropped. The gate fence lifts after one more expiry window if
+ *   admit/reject never arrive; the owner stays.
  *
  * Guards: only confirmed BACKGROUND jobs (`background === true`,
  * `state === 'running'`) participate; the generation is REQUIRED (never

+ 32 - 4
src/hooks/task-session-manager/revived-run-tracker.test.ts

@@ -1004,7 +1004,7 @@ describe('revived run tracker', () => {
       },
       undefined,
       false,
-      { handoffExpiryMs: 5, stabilizationProbeDelayMs: 0 },
+      { handoffExpiryMs: 40, stabilizationProbeDelayMs: 0 },
     );
     const gen = harness.run.generation;
 
@@ -1018,10 +1018,11 @@ describe('revived run tracker', () => {
       }),
     ).toBe(true);
 
-    // Expiry promotes the preparation into the owning run.
-    await new Promise((resolve) => setTimeout(resolve, 15));
+    // First expiry promotes; the unresolved-admission bound is a second
+    // window of the same length. Assert the fenced promoted state in
+    // between, then admit before that bound lifts.
+    await new Promise((resolve) => setTimeout(resolve, 50));
     expect(harness.tracker.isTracked('ses_child', gen)).toBe(true);
-    // Still fencing: the admission itself is unresolved.
     expect(harness.tracker.isObservationPending('ses_child', gen)).toBe(true);
     expect(harness.board.get('ses_child')?.state).toBe('running');
 
@@ -1059,6 +1060,33 @@ describe('revived run tracker', () => {
     expect(harness.tracker.isObservationPending('ses_child', gen)).toBe(false);
   });
 
+  test('handoff: unresolved admission lifts the fence after a bound without dropping the owner', async () => {
+    const harness = createHarness(
+      completedTranscript(() => false),
+      undefined,
+      false,
+      { handoffExpiryMs: 5, stabilizationProbeDelayMs: 0 },
+    );
+    const gen = harness.run.generation;
+    harness.tracker.prepareObservation({
+      taskID: 'ses_child',
+      generation: gen,
+      parentSessionID: 'parent',
+      baselineMessageID: 'baseline',
+      description: 'inspect the change',
+    });
+
+    expect(harness.tracker.settleObservationUnresolved('ses_child', gen)).toBe(
+      true,
+    );
+    expect(harness.tracker.isObservationPending('ses_child', gen)).toBe(true);
+    expect(harness.tracker.isTracked('ses_child', gen)).toBe(true);
+
+    await new Promise((resolve) => setTimeout(resolve, 20));
+    expect(harness.tracker.isObservationPending('ses_child', gen)).toBe(false);
+    expect(harness.tracker.isTracked('ses_child', gen)).toBe(true);
+  });
+
   test('handoff: prepare refuses a stale generation', () => {
     const harness = createHarness(completedTranscript(() => false));
     const gen = harness.run.generation;

+ 35 - 6
src/hooks/task-session-manager/revived-run-tracker.ts

@@ -84,9 +84,9 @@ export interface RevivedRunTracker {
    * (immediate probe, no reinstall). Reject withdraws on an explicit
    * host refusal (error envelope / capability rejection). A hung
    * admission PROMOTES the preparation into the owning run instead of
-   * dropping it `isObservationPending` stays true until admit/reject,
-   * so an `absent` verdict cannot become a stop while admission is
-   * unresolved. */
+   * dropping it. `isObservationPending` stays true until admit/reject
+   * OR a bounded unresolved-admission timer lifts the fence (owner
+   * kept) so a valid empty transcript can still confirm stopped. */
   prepareObservation(input: {
     taskID: string;
     generation: number;
@@ -100,7 +100,9 @@ export interface RevivedRunTracker {
   rejectObservation(taskID: string, generation: number): void;
   /** Unknown admission outcome (transport failed without a response):
    * the prepared ownership CONVERTS into a tracked run instead of being
-   * dropped — the host may still have accepted the replay. */
+   * dropped — the host may still have accepted the replay. The gate
+   * fence lifts after one more expiry window if admit/reject never
+   * arrive; the owner stays. */
   settleObservationUnresolved(taskID: string, generation: number): boolean;
   isObservationPending(taskID: string, generation: number): boolean;
   /** Observation-identity fence for the stop gate: a monotonic
@@ -538,8 +540,8 @@ export function createRevivedRunTracker(options: {
     // unresolved transport failure, but the ADMISSION itself is still
     // unresolved — the re-prompt may yet start, so an `absent` verdict
     // must not become a terminal stop meanwhile. The entry is cleaned
-    // only when the admission resolves (admit/reject) or an external
-    // registration supersedes it.
+    // on admit/reject, external registration, or the bounded
+    // unresolved-admission timer (owner kept).
     const pending = pendingHandoffs.get(taskID);
     return pending?.generation === generation;
   }
@@ -566,6 +568,32 @@ export function createRevivedRunTracker(options: {
     options.onRegister?.(input.taskID);
   }
 
+  /** After promotion the owner is installed but admission is still
+   * unknown. Keep fencing the gate for one more expiry window, then
+   * probe again and lift the fence WITHOUT discarding the owner — a
+   * still-empty transcript can confirm stopped, a late result still
+   * has a delivery owner. */
+  function armPromotedResolution(taskID: string, generation: number): void {
+    const pending = pendingHandoffs.get(taskID);
+    if (pending?.state !== 'promoted' || pending.generation !== generation) {
+      return;
+    }
+    if (pending.expiryTimer) clearTimeout(pending.expiryTimer);
+    pending.expiryTimer = setTimeout(() => {
+      const current = pendingHandoffs.get(taskID);
+      if (current?.state !== 'promoted' || current.generation !== generation) {
+        return;
+      }
+      void probe(taskID, generation).finally(() => {
+        const still = pendingHandoffs.get(taskID);
+        if (still?.state === 'promoted' && still.generation === generation) {
+          deleteHandoff(taskID);
+        }
+      });
+    }, handoffExpiryMs);
+    pending.expiryTimer.unref?.();
+  }
+
   /** Convert a pending preparation into the owning tracked run. Used
    * by expiry (hung admission) and unresolved transport failures: the
    * prepared owner must survive so a late acceptance — or the
@@ -597,6 +625,7 @@ export function createRevivedRunTracker(options: {
     // The re-prompt may already be persisted (admission is async): own
     // it now rather than waiting for an idle that already happened.
     void probe(taskID, pending.generation);
+    armPromotedResolution(taskID, pending.generation);
     return true;
   }