refactor(ios-snapshot): build a detached wait once, and read the retirement rule where it applies

Three reviewers in a row had to be satisfied about the same promise plumbing because it existed twice:
the discovery wait and the bridge-preparation wait each hand-rolled the timer, the caller's abort
listener, the stop listener and the cleanup that keeps a leak from costing a timer and a listener per
capture. `waitForDetachedAttempt` now implements the wait that `value()`'s contract describes, and both
owners hand it their own sleep length and their own cancellation error — which also means the invariant
is deleted-and-caught in one place instead of two.

`disableGenerationFor` said one thing about one call site, so the rule moves inline next to the set it
edits: a failed bridge retires the app generation, a bridge that is merely still building does not.

Author: thymikee

packages/platform-apple/src/detached-attempt.ts

@@ -63,6 +63,41 @@ type Attempt<Value> = {
   settled: Promise<void>;
 };
 
+/**
+ * The wait an owner hands to `value()`, built once so the promise every owner needs is the promise
+ * this module describes: sleeps `waitMs`, resolves when that sleep is spent or when `stop` says the
+ * answer arrived elsewhere, and rejects only on the caller's own abort so that stays typed.
+ *
+ * `stop` needs neither cleanup nor an already-aborted check: `value()` creates it moments before
+ * calling and aborts it in a `finally`, which releases the `{ once: true }` listener. The caller's
+ * signal outlives the wait and does have its listener removed.
+ */
+export function waitForDetachedAttempt(
+  params: Readonly<{
+    waitMs: number;
+    /** The caller's own deadline signal; a wait inside it keeps a client abort a client abort. */
+    signal: AbortSignal | undefined;
+    stop: AbortSignal;
+    /** The rejection for the caller aborting, so each owner keeps its own error type. */
+    cancelled: () => unknown;
+  }>,
+): Promise<void> {
+  const { waitMs, signal, stop, cancelled } = params;
+  return new Promise<void>((resolve, reject) => {
+    const onAbort = () => finish(() => reject(cancelled()));
+    const onStop = () => finish(resolve);
+    const timer = setTimeout(() => finish(resolve), waitMs);
+    function finish(settle: () => void): void {
+      clearTimeout(timer);
+      signal?.removeEventListener('abort', onAbort);
+      settle();
+    }
+    signal?.addEventListener('abort', onAbort, { once: true });
+    if (signal?.aborted) onAbort();
+    stop.addEventListener('abort', onStop, { once: true });
+  });
+}
+
 export function createDetachedAttempts<Value>(
   deps: Readonly<{
     waitMs: number;

packages/platform-apple/src/snapshot-route.ts

@@ -265,7 +265,11 @@ async function fallbackAfterFailure(
   disabledGenerations: Set<string>,
   cause?: unknown,
 ): Promise<SnapshotResult> {
-  disableGenerationFor(failure, failedTarget, disabledGenerations);
+  // A failed bridge is evidence about this app generation, so its captures take the runner until the
+  // generation is rebaselined. A bridge that is merely still being prepared is evidence about the
+  // daemon's build queue instead: the same generation has to be able to use it as soon as it exists,
+  // which is what let one cold host cost every later capture of a stable screen (#2491).
+  if (failure.kind !== 'preparing') disabledGenerations.add(generationKey(failedTarget));
   emitRouteDiagnostic(
     failure.code,
     { id: failedTarget.udid },
@@ -284,21 +288,6 @@ async function fallbackAfterFailure(
   );
 }
 
-/**
- * A failed bridge is evidence about this app generation, so its captures take the runner until the
- * generation is rebaselined. A bridge that is merely still being prepared is evidence about the
- * daemon's build queue instead: the same generation must be able to use it as soon as it exists,
- * which is what let one cold host cost every later capture of a stable screen (#2491).
- */
-function disableGenerationFor(
-  failure: SnapshotSourceFailure,
-  failedTarget: SimulatorSnapshotTarget,
-  disabledGenerations: Set<string>,
-): void {
-  if (failure.kind === 'preparing') return;
-  disabledGenerations.add(generationKey(failedTarget));
-}
-
 async function runFallback(
   deviceId: string,
   input: CaptureSnapshotInput,

packages/platform-apple/src/snapshot-source/deadline.ts

@@ -1,4 +1,5 @@
 import { Deadline } from '@agent-device/host-kit/retry';
+import { waitForDetachedAttempt } from '../detached-attempt.ts';
 import { snapshotSourceError } from './errors.ts';
 
 export type SnapshotSourceDeadline = Readonly<{
@@ -24,14 +25,13 @@ export function remainingSnapshotSourceMs(deadline: SnapshotSourceDeadline, code
   return Math.max(1, Math.floor(remainingMs));
 }
 
+/** A sleep nobody has asked to end early. */
+const NO_STOP = new AbortController().signal;
+
 /**
- * Sleeps inside the caller's own deadline. `stop` is for a caller that no longer needs the sleep
- * because the work it was waiting on answered elsewhere: the delay resolves instead of burning its
- * remaining budget, while an aborted `deadline` stays a typed `cancelled`.
- *
- * A stop is only ever created by the code that calls this and is always aborted by it afterwards,
- * so it needs no already-aborted check and no listener removal; the deadline's signal is the
- * caller's and does.
+ * Sleeps inside the caller's own deadline, so a client abort stays a typed `cancelled` instead of
+ * arriving as a fresh timeout. `stop` is for a caller that no longer needs the sleep because the work
+ * it was waiting on answered elsewhere: the delay ends without burning the rest of its budget.
  */
 export async function waitForSnapshotSourceDelay(
   deadline: SnapshotSourceDeadline,
@@ -40,22 +40,10 @@ export async function waitForSnapshotSourceDelay(
   stop?: AbortSignal,
 ): Promise<void> {
   const delayMs = Math.min(requestedMs, remainingSnapshotSourceMs(deadline, code));
-  await new Promise<void>((resolve, reject) => {
-    let settled = false;
-    const timer = setTimeout(() => finish(resolve), delayMs);
-    const onAbort = () => {
-      finish(() => reject(snapshotSourceError('cancelled', 'abort-signal')));
-    };
-    const onStop = () => finish(resolve);
-    const finish = (action: () => void) => {
-      if (settled) return;
-      settled = true;
-      clearTimeout(timer);
-      deadline.signal?.removeEventListener('abort', onAbort);
-      action();
-    };
-    deadline.signal?.addEventListener('abort', onAbort, { once: true });
-    if (deadline.signal?.aborted) onAbort();
-    stop?.addEventListener('abort', onStop, { once: true });
+  await waitForDetachedAttempt({
+    waitMs: delayMs,
+    signal: deadline.signal,
+    stop: stop ?? NO_STOP,
+    cancelled: () => snapshotSourceError('cancelled', 'abort-signal'),
   });
 }

packages/platform-apple/src/snapshot-target.ts

@@ -1,6 +1,6 @@
 import type { DeviceInfo } from '@agent-device/kernel/device';
 import { AppError } from '@agent-device/kernel/errors';
-import { createDetachedAttempts } from './detached-attempt.ts';
+import { createDetachedAttempts, waitForDetachedAttempt } from './detached-attempt.ts';
 import { runSimctl } from './core/apps-simctl.ts';
 import { readSnapshotTargetProcessStartTime } from './snapshot-process.ts';
 
@@ -59,40 +59,13 @@ export function createSimulatorSnapshotTargetResolver(): SimulatorSnapshotTarget
         targets.set(key, target);
         return target;
       },
-      wait: (waitMs, stop) => waitForDiscoveryAttempt(waitMs, signal, stop),
+      wait: (waitMs, stop) =>
+        waitForDetachedAttempt({ waitMs, signal, stop, cancelled: () => signal.reason }),
       pending: () => targetError('simulator-target-discovery-pending', device, appBundleId),
     });
   };
 }
 
-/**
- * One caller's wait for a discovery it did not start. Resolving is the wait being spent, not the
- * discovery failing: a client abort rejects with its own reason so it stays typed `cancelled`, while
- * `stop` means this caller already has its answer and is only letting go of the timer.
- *
- * The stop needs no cleanup here and no already-aborted check: `value()` creates it moments before
- * calling this and aborts it in a `finally`, so the listener is gone once that abort fires. The
- * caller's signal outlives this wait and does need its listener removed.
- */
-function waitForDiscoveryAttempt(
-  waitMs: number,
-  signal: AbortSignal,
-  stop: AbortSignal,
-): Promise<void> {
-  return new Promise<void>((resolve, reject) => {
-    const onAbort = () => finish(() => reject(signal.reason));
-    const onStop = () => finish(resolve);
-    const timer = setTimeout(() => finish(resolve), waitMs);
-    function finish(settle: () => void): void {
-      clearTimeout(timer);
-      signal.removeEventListener('abort', onAbort);
-      settle();
-    }
-    signal.addEventListener('abort', onAbort, { once: true });
-    stop.addEventListener('abort', onStop, { once: true });
-  });
-}
-
 async function resolveSimulatorSnapshotTarget(
   device: DeviceInfo,
   appBundleId: string,