test(ios-snapshot): prove the discovery wait stops when the discovery settles

The stop path inside `waitForDiscoveryAttempt` had no test: the listener registration, the listener
removal and the already-aborted check could each be deleted with every suite green.

`snapshot-target.test.ts` now runs a deferred spawn through `createSimulatorSnapshotTargetResolver`,
lets a second caller join the pending discovery, settles it, and asserts on the two things a leaked
wait costs — a timer still pending and a listener still on the caller's `AbortSignal` — with fake
timers, so neither is a timing race. `deadline.test.ts` measures the same two properties for the
preparation owner and drops its already-aborted case.

That case was one of the things the review asked about, and it is unreachable rather than untested:
`value()` creates the stop moments before calling and aborts it in a `finally`, so the check can never
be true and the stop's `{ once: true }` listener is always released by that abort. Both waits now
drop the check and the redundant removal, and keep the cleanup that does matter, which is the
caller's own signal: it outlives the wait, and forgetting it would leave a listener per capture.

Author: thymikee

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

@@ -1,46 +1,49 @@
-import assert from 'node:assert/strict';
-import { test } from 'vitest';
+import { expect, test, vi } from 'vitest';
 import { createSnapshotSourceDeadline, waitForSnapshotSourceDelay } from './deadline.ts';
 import { SnapshotSourceError } from './errors.ts';
 
 const WAIT_CODE = 'bridge-preparation-pending';
 
 test('a stopped delay returns without spending the rest of the deadline', async () => {
-  const stop = new AbortController();
-  const deadline = createSnapshotSourceDeadline(60_000, undefined);
-  const waiting = waitForSnapshotSourceDelay(deadline, 60_000, WAIT_CODE, stop.signal);
+  vi.useFakeTimers();
+  try {
+    const stop = new AbortController();
+    const deadline = createSnapshotSourceDeadline(60_000, undefined);
+    let settled = false;
+    const waiting = waitForSnapshotSourceDelay(deadline, 60_000, WAIT_CODE, stop.signal).then(
+      () => {
+        settled = true;
+      },
+    );
 
-  const startedAt = Date.now();
-  stop.abort();
-  await waiting;
-  assert.ok(Date.now() - startedAt < 5_000, 'a stopped wait does not sleep out its budget');
-});
-
-test('a delay started with its stop already aborted does not sleep', async () => {
-  const stop = new AbortController();
-  stop.abort();
-  const deadline = createSnapshotSourceDeadline(60_000, undefined);
+    await vi.advanceTimersByTimeAsync(0);
+    expect(settled).toBe(false);
+    stop.abort();
+    await waiting;
 
-  const startedAt = Date.now();
-  await waitForSnapshotSourceDelay(deadline, 60_000, WAIT_CODE, stop.signal);
-  assert.ok(Date.now() - startedAt < 5_000);
+    expect(settled).toBe(true);
+    expect(vi.getTimerCount()).toBe(0);
+  } finally {
+    vi.useRealTimers();
+  }
 });
 
 test('an aborted caller signal stays typed cancellation next to a stop', async () => {
-  const caller = new AbortController();
-  const deadline = createSnapshotSourceDeadline(60_000, caller.signal);
-  const waiting = waitForSnapshotSourceDelay(
-    deadline,
-    60_000,
-    WAIT_CODE,
-    new AbortController().signal,
-  );
+  vi.useFakeTimers();
+  try {
+    const caller = new AbortController();
+    const stop = new AbortController();
+    const deadline = createSnapshotSourceDeadline(60_000, caller.signal);
+    const waiting = waitForSnapshotSourceDelay(deadline, 60_000, WAIT_CODE, stop.signal);
 
-  caller.abort();
-  await assert.rejects(waiting, (error: unknown) => {
-    assert.ok(error instanceof SnapshotSourceError);
-    assert.equal(error.failureKind, 'cancelled');
-    assert.equal(error.failureCode, 'abort-signal');
-    return true;
-  });
+    caller.abort();
+    await expect(waiting).rejects.toBeInstanceOf(SnapshotSourceError);
+    await expect(waiting).rejects.toMatchObject({
+      failureKind: 'cancelled',
+      failureCode: 'abort-signal',
+    });
+    expect(vi.getTimerCount()).toBe(0);
+  } finally {
+    vi.useRealTimers();
+  }
 });

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

@@ -28,6 +28,10 @@ export function remainingSnapshotSourceMs(deadline: SnapshotSourceDeadline, code
  * 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.
  */
 export async function waitForSnapshotSourceDelay(
   deadline: SnapshotSourceDeadline,
@@ -48,10 +52,8 @@ export async function waitForSnapshotSourceDelay(
       settled = true;
       clearTimeout(timer);
       deadline.signal?.removeEventListener('abort', onAbort);
-      stop?.removeEventListener('abort', onStop);
       action();
     };
-    if (stop?.aborted) return finish(resolve);
     deadline.signal?.addEventListener('abort', onAbort, { once: true });
     if (deadline.signal?.aborted) onAbort();
     stop?.addEventListener('abort', onStop, { once: true });

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

@@ -1,3 +1,4 @@
+import { listenerCount } from 'node:events';
 import { expect, test, vi } from 'vitest';
 import type { DeviceInfo } from '@agent-device/kernel/device';
 import { createLocalAppleToolProvider, withAppleToolProvider } from './core/tool-provider.ts';
@@ -140,6 +141,11 @@ test('an aborted request cannot reuse a cached target', async () => {
   });
 });
 
+/** Node counts EventTarget listeners through the same entry point it uses for emitters. */
+function abortListeners(signal: AbortSignal): number {
+  return listenerCount(signal as unknown as Parameters<typeof listenerCount>[0], 'abort');
+}
+
 function deferredSpawn(fixture: ReturnType<typeof targetFixture>) {
   let release!: () => void;
   const released = new Promise<void>((resolve) => {
@@ -263,3 +269,28 @@ test('a failed runtime probe does not release the slot while the launch-job prob
     vi.useRealTimers();
   }
 });
+
+test('a discovery that settles leaves no timer or abort listener behind for its waiters', async () => {
+  const fixture = targetFixture();
+  const release = deferredSpawn(fixture);
+  vi.useFakeTimers();
+  try {
+    await withAppleToolProvider(fixture.provider, async () => {
+      const starting = new AbortController();
+      const joining = new AbortController();
+      const first = fixture.resolve(ios, app, starting.signal);
+      const second = fixture.resolve(ios, app, joining.signal);
+      await vi.advanceTimersByTimeAsync(0);
+
+      release();
+      await vi.advanceTimersByTimeAsync(0);
+      await Promise.all([first, second]);
+
+      expect(abortListeners(starting.signal)).toBe(0);
+      expect(abortListeners(joining.signal)).toBe(0);
+      expect(vi.getTimerCount()).toBe(0);
+    });
+  } finally {
+    vi.useRealTimers();
+  }
+});

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

@@ -69,6 +69,10 @@ export function createSimulatorSnapshotTargetResolver(): SimulatorSnapshotTarget
  * 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,
@@ -82,10 +86,8 @@ function waitForDiscoveryAttempt(
     function finish(settle: () => void): void {
       clearTimeout(timer);
       signal.removeEventListener('abort', onAbort);
-      stop.removeEventListener('abort', onStop);
       settle();
     }
-    if (stop.aborted) return finish(resolve);
     signal.addEventListener('abort', onAbort, { once: true });
     stop.addEventListener('abort', onStop, { once: true });
   });