fix: sleep past the settle quiet deadline instead of onto it (#1306)

src/commands/interaction/runtime/selector-read-stable.test.ts

@@ -43,6 +43,133 @@ test('runtime wait stable settles after two unchanged captures', async () => {
   assert.equal(captures, 3);
 });
 
+// A clock whose sleep lands `undershootMs` short of what was asked, modelling
+// the real defect: `setTimeout(n)` can advance `Date.now()` by only n-1,
+// because libuv times the sleep on the monotonic loop clock while `now()` reads
+// the wall clock. It yields to the event loop like a real sleep does, so a loop
+// that fails to terminate fails the test on its timeout rather than starving
+// the runner on microtasks.
+function createUndershootingClock(undershootMs = 1): {
+  now: () => number;
+  sleep: (ms: number) => Promise<void>;
+} {
+  let elapsed = 0;
+  return {
+    now: () => elapsed,
+    sleep: async (ms: number) => {
+      elapsed += Math.max(0, ms - undershootMs);
+      await new Promise((resolve) => {
+        setImmediate(resolve);
+      });
+    },
+  };
+}
+
+function createStableCaptureDevice(clock: {
+  now: () => number;
+  sleep: (ms: number) => Promise<void>;
+}) {
+  const snapshot = selectorReadSnapshot();
+  const counter = { captures: 0 };
+  const device = createAgentDevice({
+    backend: {
+      platform: 'ios',
+      captureSnapshot: async () => {
+        counter.captures += 1;
+        return { snapshot };
+      },
+    } satisfies AgentDeviceBackend,
+    artifacts: createLocalArtifactAdapter(),
+    sessions: createMemorySessionStore([{ name: 'default', snapshot }]),
+    policy: localCommandPolicy(),
+    clock,
+  });
+  return { device, counter };
+}
+
+test('runtime wait stable settles at the second capture even when the sleep undershoots', async () => {
+  // A 300ms quiet window equals the poll cadence, so the second capture is the
+  // one that decides `settled` — and a sleep that lands 1ms short must not
+  // change the verdict (#1306).
+  const { device, counter } = createStableCaptureDevice(createUndershootingClock());
+
+  const result = await device.selectors.wait({
+    session: 'default',
+    target: { kind: 'stable', quietMs: 300, timeoutMs: 10_000 },
+  });
+
+  assert.equal(result.kind, 'stable');
+  if (result.kind === 'stable') assert.equal(result.captures, 2);
+  assert.equal(counter.captures, 2);
+});
+
+test('runtime wait stable still takes its settling capture when the budget barely clears the window', async () => {
+  // One millisecond of budget past the quiet window is enough to settle, and
+  // the deadline-aware sleep must not spend it on the epsilon: overshooting the
+  // deadline would drop the settling capture the plain cadence would have taken.
+  const { device, counter } = createStableCaptureDevice(createFakeClock());
+
+  const result = await device.selectors.wait({
+    session: 'default',
+    target: { kind: 'stable', quietMs: 300, timeoutMs: 301 },
+  });
+
+  assert.equal(result.kind, 'stable');
+  if (result.kind === 'stable') {
+    assert.equal(result.captures, 2);
+    assert.ok(
+      result.waitedMs <= 301,
+      `waitedMs must stay within the budget, got ${result.waitedMs}`,
+    );
+  }
+  assert.equal(counter.captures, 2);
+});
+
+test('runtime wait stable cannot settle when the budget only reaches the quiet deadline', async () => {
+  // The other side of the boundary: the window has to ELAPSE inside the budget,
+  // so a budget equal to it leaves no instant where two captures span it. The
+  // deadline-aware sleep must neither manufacture a settle here nor run past
+  // the budget once no further capture can land inside it.
+  const clock = createFakeClock();
+  const { device } = createStableCaptureDevice(clock);
+
+  await assert.rejects(
+    () =>
+      device.selectors.wait({
+        session: 'default',
+        target: { kind: 'stable', quietMs: 300, timeoutMs: 300 },
+      }),
+    (error: unknown) =>
+      error instanceof Error &&
+      (error as { details?: { reason?: string } }).details?.reason === 'wait_stable_timeout',
+  );
+  assert.ok(clock.now() <= 300, `loop must not run past its budget, elapsed ${clock.now()}ms`);
+});
+
+test('runtime wait stable uses the final budget without spinning when sleep undershoots', async () => {
+  // The two hazards combined: a budget that leaves a 1ms landing window, and a
+  // sleep that loses 1ms to skew. Asking for that 1ms buys no time, so a loop
+  // that trusts the sleep to carry it past the deadline never gets there and
+  // hammers the device with captures instead. Spending the full 2ms advances
+  // this clock to 300ms and earns the valid settling capture; an exact clock
+  // would land on the 301ms deadline and exit without another capture.
+  const clock = createUndershootingClock();
+  const { device, counter } = createStableCaptureDevice(clock);
+
+  const result = await device.selectors.wait({
+    session: 'default',
+    target: { kind: 'stable', quietMs: 300, timeoutMs: 301 },
+  });
+
+  assert.equal(result.kind, 'stable');
+  if (result.kind === 'stable') {
+    assert.equal(result.captures, 3);
+    assert.ok(result.waitedMs <= 301, `waitedMs must stay within budget, got ${result.waitedMs}`);
+  }
+  assert.equal(counter.captures, 3, `loop must not spin, took ${counter.captures} captures`);
+  assert.ok(clock.now() <= 301, `loop must not run past its budget, elapsed ${clock.now()}ms`);
+});
+
 test('runtime wait stable hints when it settles on a nearly-empty tree', async () => {
   const tinySnapshot = makeSnapshotState([
     {

src/commands/interaction/runtime/stable-capture.ts

@@ -19,6 +19,12 @@ import {
  */
 
 const STABLE_POLL_INTERVAL_MS = 300;
+const STABLE_MIN_POLL_MS = 25;
+// Wake PAST the quiet deadline rather than exactly on it. `setTimeout(n)` can
+// advance `Date.now()` by only n-1 — libuv times the sleep on the monotonic
+// loop clock while `now()` reads the wall clock — so a capture landing on the
+// deadline decides `settled` by sub-millisecond skew rather than by the UI.
+const QUIET_DEADLINE_EPSILON_MS = 2;
 export const DEFAULT_STABLE_QUIET_MS = 500;
 export const DEFAULT_STABLE_TIMEOUT_MS = 10_000;
 // Below this node count a settled tree is suspicious: real app surfaces have
@@ -53,7 +59,7 @@ export async function runStableCaptureLoop(
   // Cadence derives from the quiet window (never slower than the default
   // poll): a caller asking for a 50ms quiet window should not be forced onto a
   // 300ms grid — and tests inject the budget instead of waiting real time.
-  const pollMs = Math.min(STABLE_POLL_INTERVAL_MS, Math.max(25, quietMs));
+  const pollMs = Math.min(STABLE_POLL_INTERVAL_MS, Math.max(STABLE_MIN_POLL_MS, quietMs));
   let captures = 0;
   let lastDigest: string | undefined;
   let lastNodeCount = 0;
@@ -89,7 +95,15 @@ export async function runStableCaptureLoop(
       quietSinceMs = nowMs;
       lastDigest = digest;
       lastNodeCount = capture.snapshot.nodes.length;
-      await sleep(runtime, pollMs);
+      const recoveryDelayMs = stableCaptureDelayMs({
+        nowMs,
+        quietSinceMs,
+        quietMs,
+        pollMs,
+        deadlineMs,
+      });
+      if (recoveryDelayMs <= 0) break;
+      await sleep(runtime, recoveryDelayMs);
       continue;
     }
     if (digest !== lastDigest) {
@@ -106,7 +120,9 @@ export async function runStableCaptureLoop(
         lastCapture,
       };
     }
-    await sleep(runtime, pollMs);
+    const delayMs = stableCaptureDelayMs({ nowMs, quietSinceMs, quietMs, pollMs, deadlineMs });
+    if (delayMs <= 0) break;
+    await sleep(runtime, delayMs);
   }
   return {
     settled: false,
@@ -122,6 +138,44 @@ function isPrivateAxRecovery(verdict: SnapshotQualityVerdict | undefined): boole
   return verdict?.state === 'recovered' && verdict.backend === 'private-ax';
 }
 
+/**
+ * How long to wait before the next capture, or 0 when there is no wait worth
+ * taking and the loop should stop.
+ *
+ * While the quiet deadline is still further away than one poll, keep the
+ * cadence so a change is noticed promptly. Once it is within reach, sleep to
+ * just past it instead — the capture that decides `settled` then always spans
+ * the window, rather than landing on the boundary where clock skew picks the
+ * answer (#1306).
+ *
+ * The loop only runs again while `now < deadline`. Normally the wake-up must
+ * land strictly inside the budget to leave room for another capture. At the
+ * final skew-sized boundary, however, requesting the full remaining budget is
+ * useful: an exact clock lands on the deadline and exits, while an undershooting
+ * clock advances inside the deadline and gets the settling capture. Never
+ * request less than the epsilon — a 1ms sleep can advance the wall clock by 0
+ * and spin forever under the skew this function exists to absorb.
+ */
+function stableCaptureDelayMs(params: {
+  nowMs: number;
+  quietSinceMs: number;
+  quietMs: number;
+  pollMs: number;
+  deadlineMs: number;
+}): number {
+  const remainingQuietMs = params.quietSinceMs + params.quietMs - params.nowMs;
+  const cadenceMs =
+    remainingQuietMs > params.pollMs
+      ? params.pollMs
+      : Math.max(STABLE_MIN_POLL_MS, remainingQuietMs + QUIET_DEADLINE_EPSILON_MS);
+  const remainingBudgetMs = params.deadlineMs - params.nowMs;
+  const lastUsefulWakeMs = remainingBudgetMs - 1;
+  if (lastUsefulWakeMs < QUIET_DEADLINE_EPSILON_MS) {
+    return remainingBudgetMs >= QUIET_DEADLINE_EPSILON_MS ? remainingBudgetMs : 0;
+  }
+  return Math.min(cadenceMs, lastUsefulWakeMs);
+}
+
 // Intentionally does not update the session snapshot: the stable loop captures
 // an interactive-only tree purely as a settle signal, and overwriting the
 // session's richer cached snapshot with the filtered tree would degrade