fix(test): tolerate dead session in iOS e2e full-tier cleanup (#1548)

* fix(test): tolerate dead session in iOS e2e cleanup

full:device-lifecycle reboots the simulator, and whether the daemon
session survives that is environment-sensitive: it does on CI but not
locally, so every all-green local full-tier run ended red in cleanup
with all three retries of each step failing ('permission setting
requires an active app in session' / 'No active session').

Two layers:
- finalizeLiveRun re-checks sessionExists instead of short-circuiting
  on sessionOpen, so a session that died mid-run skips cleanup entirely.
- cleanupSession treats SESSION_NOT_FOUND and the appless-session
  INVALID_ARGS failure as already-clean instead of burning retries.

Other cleanup failures still exhaust three attempts and fail loudly.

* fix(test): narrow appless-session cleanup guard to the mic-permission step

Scope sessionAlreadyClean's INVALID_ARGS tolerance to the microphone-
permission reset step and its exact known message instead of matching
any cleanup step whose message contains "requires an active app in
session" — that substring is also thrown by the unrelated location
setting, so the old check could have hidden a real failure there.

Extract the per-step retry policy into an exported retryCleanupStep so
it's unit-testable without spawning the CLI, and add a deterministic
regression (test/integration/ios-simulator-e2e-cleanup.test.ts)
covering: a dead session (SESSION_NOT_FOUND) stops retrying on any
step, the known mic-permission appless response stops retrying, and a
different INVALID_ARGS (wrong step or wrong message) still exhausts
all three retries and fails.

* test(ios-e2e): cover the finalization cleanup-gate decision

Extract the sessionOpen re-check + conditional cleanupSession call out
of finalizeLiveRun into an exported finalizeSessionCleanup(context,
runSessionExists, runCleanupSession) — same behavior, now driven by
injected callbacks instead of the module-level runStep-backed
bindings, so it's unit-testable without spawning the CLI.

Add two deterministic cases: sessionOpen starts true and the final
sessionExists() resolves false -> cleanupSession is never invoked;
sessionOpen true and sessionExists() resolves true -> cleanupSession
runs (the live path). Counterfactual (reverting the recheck to the old
`sessionOpen || sessionExists(...)` form) turns the first case red as
expected.

Author: thymikee

test/integration/ios-simulator-e2e-cleanup.test.ts

@@ -0,0 +1,160 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+
+import type { CliJsonResult } from './cli-json.ts';
+import type { LiveContext } from './ios-simulator-e2e/live-harness.ts';
+import { retryCleanupStep } from './ios-simulator-e2e/live-harness.ts';
+import { finalizeSessionCleanup } from './ios-simulator-e2e/live-runner.ts';
+
+// Deterministic regression for the retry/guard policy behind #1548: a full-tier iOS
+// e2e run's cleanup must tolerate a dead session and the known appless mic-permission
+// reset without swallowing an unrelated failure. `retryCleanupStep` runs the exact
+// per-step policy `cleanupSession` uses, driven here by a scripted `runAttempt` instead
+// of the real CLI subprocess, so these cases run in milliseconds with no simulator.
+
+const MIC_STEP = 'reset microphone permission';
+const OTHER_STEP = 'restore portrait orientation';
+const MIC_APPLESS_MESSAGE = 'permission setting requires an active app in session';
+
+function invalidArgsResult(message: string): CliJsonResult {
+  return { json: { error: { code: 'INVALID_ARGS', message } }, status: 1, stderr: '', stdout: '' };
+}
+
+function sessionNotFoundResult(): CliJsonResult {
+  return {
+    json: { error: { code: 'SESSION_NOT_FOUND', message: 'No active session' } },
+    status: 1,
+    stderr: '',
+    stdout: '',
+  };
+}
+
+// retryCleanupStep sleeps 500ms between attempts. Drive fake timers instead of waiting
+// real time: repeatedly flush the microtask queue and advance the mocked clock until
+// the retry promise settles.
+async function drainRetry(
+  t: { mock: { timers: { tick: (ms: number) => void } } },
+  promise: Promise<unknown>,
+): Promise<unknown> {
+  let settled = false;
+  promise.finally(() => {
+    settled = true;
+  });
+  while (!settled) {
+    await new Promise((resolve) => setImmediate(resolve));
+    t.mock.timers.tick(500);
+  }
+  return promise;
+}
+
+test('a dead session (SESSION_NOT_FOUND) skips cleanup without error, on any step', async () => {
+  let attempts = 0;
+  const failure = await retryCleanupStep(MIC_STEP, async () => {
+    attempts += 1;
+    return sessionNotFoundResult();
+  });
+  assert.equal(failure, undefined);
+  assert.equal(attempts, 1, 'should not retry once the session is confirmed gone');
+});
+
+test('the known mic-permission appless response stops retrying immediately', async () => {
+  let attempts = 0;
+  const failure = await retryCleanupStep(MIC_STEP, async () => {
+    attempts += 1;
+    return invalidArgsResult(MIC_APPLESS_MESSAGE);
+  });
+  assert.equal(failure, undefined);
+  assert.equal(attempts, 1, 'should not retry the known appless response');
+});
+
+test('a different INVALID_ARGS still fails after exhausting retries', async (t) => {
+  // Same message, wrong step: proves the guard is scoped to the mic-permission reset
+  // and does not tolerate the identical string on another step.
+  t.mock.timers.enable({ apis: ['setTimeout'] });
+  let attempts = 0;
+  const failure = await drainRetry(
+    t,
+    retryCleanupStep(OTHER_STEP, async (attempt) => {
+      attempts += 1;
+      if (attempt < 3) return invalidArgsResult(MIC_APPLESS_MESSAGE);
+      // Mirrors runStep(..., { allowFailure: false }) on the final attempt: it throws
+      // instead of returning a failed result.
+      throw new Error(`cleanup: ${OTHER_STEP} (attempt 3) failed`);
+    }),
+  );
+  assert.ok(failure instanceof Error, `expected a propagated failure, got ${String(failure)}`);
+  assert.equal(attempts, 3, 'should exhaust all three attempts');
+});
+
+test('a different INVALID_ARGS message on the mic-permission step still fails after retries', async (t) => {
+  // Same step, a message sharing the "requires an active app in session" suffix with
+  // the location-setting call site (app-settings.ts): proves the match is the exact
+  // known string, not any INVALID_ARGS message that happens to overlap it.
+  t.mock.timers.enable({ apis: ['setTimeout'] });
+  let attempts = 0;
+  const failure = await drainRetry(
+    t,
+    retryCleanupStep(MIC_STEP, async (attempt) => {
+      attempts += 1;
+      if (attempt < 3)
+        return invalidArgsResult('location setting requires an active app in session');
+      throw new Error(`cleanup: ${MIC_STEP} (attempt 3) failed`);
+    }),
+  );
+  assert.ok(failure instanceof Error, `expected a propagated failure, got ${String(failure)}`);
+  assert.equal(attempts, 3, 'should exhaust all three attempts');
+});
+
+// Deterministic regression for finalizeSessionCleanup: the other half of #1548, which
+// decides whether cleanup runs at all. A minimal LiveContext fixture; only sessionOpen
+// is read by the decision, the rest exists to satisfy the type.
+function fixtureContext(sessionOpen: boolean): LiveContext {
+  return {
+    appId: 'com.example.fixture',
+    appPath: '/fixture.app',
+    artifactDir: '/tmp/fixture-artifacts',
+    behaviorEvidence: {},
+    commandEvidence: {},
+    completedScenarios: [],
+    currentScenario: 'full:device-lifecycle',
+    env: {},
+    session: 'fixture-session',
+    sessionOpen,
+    stateDir: '/tmp/fixture-state',
+    startedAtMs: Date.now(),
+    stepHistory: [],
+    tier: 'full',
+    timings: [],
+    udid: 'fixture-udid',
+  };
+}
+
+test('sessionOpen=true, final sessionExists=false: cleanup is never invoked', async () => {
+  let cleanupCalls = 0;
+  const context = fixtureContext(true);
+  const cleanupError = await finalizeSessionCleanup(
+    context,
+    async () => false,
+    async () => {
+      cleanupCalls += 1;
+    },
+  );
+  assert.equal(cleanupCalls, 0, 'cleanupSession must not run once the session is confirmed gone');
+  assert.equal(context.sessionOpen, false, 'sessionOpen should reflect the re-check, not the flag');
+  assert.equal(cleanupError, undefined);
+});
+
+test('sessionOpen=true, final sessionExists=true: cleanup is invoked (the live path)', async () => {
+  let cleanupCalls = 0;
+  const context = fixtureContext(true);
+  const cleanupError = await finalizeSessionCleanup(
+    context,
+    async () => true,
+    async () => {
+      cleanupCalls += 1;
+    },
+  );
+  assert.equal(cleanupCalls, 1, 'cleanupSession must run while the session is still live');
+  assert.equal(context.sessionOpen, true);
+  assert.equal(cleanupError, undefined);
+});

test/integration/ios-simulator-e2e/live-harness.ts

@@ -3,6 +3,7 @@ import fs from 'node:fs';
 import path from 'node:path';
 
 import { resolveDaemonPaths } from '../../../src/daemon/config.ts';
+import type { CliJsonResult } from '../cli-json.ts';
 import {
   createLiveDeviceContext,
   createLiveDeviceHarness,
@@ -75,33 +76,66 @@ export function verifyNestedReplayCommand(
   harness.verifyNestedCommand(context, command, executedVia, evidence);
 }
 
+// Shared between the step list and the guard below so the two can't drift apart.
+const MICROPHONE_PERMISSION_RESET_STEP = 'reset microphone permission';
+
 export async function cleanupSession(context: LiveContext): Promise<void> {
   const failures: unknown[] = [];
   const cleanupSteps: Array<[string, string[]]> = [];
   if (context.tier === 'full') {
     cleanupSteps.push(
-      ['reset microphone permission', ['settings', 'permission', 'reset', 'microphone']],
+      [MICROPHONE_PERMISSION_RESET_STEP, ['settings', 'permission', 'reset', 'microphone']],
       ['restore light appearance', ['settings', 'appearance', 'light']],
       ['restore portrait orientation', ['orientation', 'portrait']],
     );
   }
   cleanupSteps.push(['close fixture session', ['close']]);
   for (const [step, args] of cleanupSteps) {
-    for (let attempt = 1; attempt <= 3; attempt += 1) {
-      try {
-        const result = await runStep(context, `cleanup: ${step} (attempt ${attempt})`, args, {
-          allowFailure: attempt < 3,
-        });
-        if (result.status === 0) break;
-        await new Promise((resolve) => setTimeout(resolve, 500));
-      } catch (error) {
-        failures.push(error);
-        break;
-      }
-    }
+    const failure = await retryCleanupStep(step, (attempt) =>
+      runStep(context, `cleanup: ${step} (attempt ${attempt})`, args, {
+        allowFailure: attempt < 3,
+      }),
+    );
+    if (failure !== undefined) failures.push(failure);
   }
   if (failures.length === 0) return;
   const errorPath = path.join(context.artifactDir, 'cleanup-error.txt');
   fs.writeFileSync(errorPath, failures.map(String).join('\n\n'));
   throw new AggregateError(failures, `iOS E2E cleanup failed; details: ${errorPath}`);
 }
+
+/**
+ * Runs one cleanup step's 3-attempt retry policy: success or an already-clean session
+ * stops immediately, anything else waits and retries. `runAttempt` mirrors the real
+ * `runStep(..., { allowFailure: attempt < 3 })` contract, including that the final
+ * attempt throws instead of returning a failed result. Exported so the retry/guard
+ * behavior is unit-testable without spawning the CLI (see
+ * `test/integration/ios-simulator-e2e-cleanup.test.ts`).
+ */
+export async function retryCleanupStep(
+  step: string,
+  runAttempt: (attempt: number) => Promise<CliJsonResult>,
+): Promise<unknown> {
+  for (let attempt = 1; attempt <= 3; attempt += 1) {
+    try {
+      const result = await runAttempt(attempt);
+      if (result.status === 0 || sessionAlreadyClean(step, result)) return undefined;
+      await new Promise((resolve) => setTimeout(resolve, 500));
+    } catch (error) {
+      return error;
+    }
+  }
+  return undefined;
+}
+
+// SESSION_NOT_FOUND: nothing left to reset, any step. INVALID_ARGS: only the
+// mic-permission reset needs an app bundle and app-settings.ts has no reason code for
+// it, so we match its exact message — scoped to this step so it can't hide another failure.
+function sessionAlreadyClean(step: string, result: CliJsonResult): boolean {
+  if (result.json?.error?.code === 'SESSION_NOT_FOUND') return true;
+  return (
+    step === MICROPHONE_PERMISSION_RESET_STEP &&
+    result.json?.error?.code === 'INVALID_ARGS' &&
+    result.json?.error?.message === 'permission setting requires an active app in session'
+  );
+}

test/integration/ios-simulator-e2e/live-runner.ts

@@ -72,24 +72,41 @@ async function executeLiveScenarios(context: LiveContext): Promise<void> {
 }
 
 async function finalizeLiveRun(context: LiveContext): Promise<unknown> {
+  let cleanupError = await finalizeSessionCleanup(context, sessionExists, cleanupSession);
+  try {
+    writeCoverageReport(context);
+  } catch (error) {
+    cleanupError = combineErrors(cleanupError, error, 'cleanup and coverage reporting failed');
+  }
+  return cleanupError;
+}
+
+/**
+ * Decides whether session-scoped cleanup runs: full:device-lifecycle reboots the
+ * simulator and the session lease does not survive that in every environment, so this
+ * re-checks session existence (the daemon's session list is authoritative at finalize
+ * time) instead of trusting `sessionOpen` accumulated during the run, and only invokes
+ * cleanup when a session remains. Exported so the decision is unit-testable without
+ * spawning the CLI (see test/integration/ios-simulator-e2e-cleanup.test.ts).
+ */
+export async function finalizeSessionCleanup(
+  context: LiveContext,
+  runSessionExists: (context: LiveContext) => Promise<boolean>,
+  runCleanupSession: (context: LiveContext) => Promise<void>,
+): Promise<unknown> {
   let cleanupError: unknown;
   try {
-    context.sessionOpen = context.sessionOpen || (await sessionExists(context));
+    context.sessionOpen = await runSessionExists(context);
   } catch (error) {
     cleanupError = error;
   }
   if (context.sessionOpen) {
     try {
-      await cleanupSession(context);
+      await runCleanupSession(context);
     } catch (error) {
       cleanupError = combineErrors(cleanupError, error, 'session inspection and cleanup failed');
     }
   }
-  try {
-    writeCoverageReport(context);
-  } catch (error) {
-    cleanupError = combineErrors(cleanupError, error, 'cleanup and coverage reporting failed');
-  }
   return cleanupError;
 }