fix(android): back off a timed-out snapshot helper session and bound content re-captures

packages/platform-android/src/__tests__/snapshot-content-recapture.test.ts

@@ -0,0 +1,100 @@
+import { afterEach, beforeEach, test, vi } from 'vitest';
+import assert from 'node:assert/strict';
+
+vi.mock('../adb.ts', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../adb.ts')>();
+  return { ...actual, sleep: vi.fn(async () => {}) };
+});
+
+import type { DeviceInfo } from '@agent-device/kernel/device';
+import { AppError } from '@agent-device/kernel/errors';
+import { sleep } from '../adb.ts';
+import { snapshotAndroid } from '../snapshot.ts';
+import { resetAndroidSnapshotHelperInstallCache } from '../snapshot-helper-install.ts';
+import { resetAndroidSnapshotHelperSessions } from '../snapshot-helper-session-lifecycle.ts';
+import { ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT } from './test-utils/android-snapshot-helper.ts';
+import {
+  androidSystemWindowOnlyXml,
+  createPersistentSnapshotHelperProvider,
+  type FakeAndroidProcess,
+} from './snapshot-helper-session.fixtures.ts';
+
+const device: DeviceInfo = {
+  platform: 'android',
+  id: 'emulator-5554',
+  name: 'Pixel',
+  kind: 'emulator',
+  booted: true,
+};
+
+beforeEach(async () => {
+  await resetAndroidSnapshotHelperSessions();
+  resetAndroidSnapshotHelperInstallCache();
+});
+
+afterEach(async () => {
+  vi.restoreAllMocks();
+  await resetAndroidSnapshotHelperSessions();
+});
+
+/**
+ * Captures of a busy screen that each answer with system chrome only, after `captureCostMs`.
+ * Every sleep advances the clock by the time it was asked to wait.
+ */
+async function captureBusyScreen(captureCostMs: number): Promise<{
+  error: unknown;
+  sessionCaptures: number;
+}> {
+  let clockOffsetMs = 0;
+  const realNow = Date.now.bind(Date);
+  vi.spyOn(Date, 'now').mockImplementation(() => realNow() + clockOffsetMs);
+  vi.mocked(sleep).mockImplementation(async (ms: number) => {
+    clockOffsetMs += ms;
+  });
+  let sessionCaptures = 0;
+  const provider = createPersistentSnapshotHelperProvider({
+    calls: [],
+    spawnArgs: [],
+    processes: [] as FakeAndroidProcess[],
+    sessionXml: () => {
+      sessionCaptures += 1;
+      clockOffsetMs += captureCostMs;
+      return androidSystemWindowOnlyXml();
+    },
+  });
+  const error = await snapshotAndroid(device, {
+    helperAdb: provider,
+    helperArtifact: ANDROID_SNAPSHOT_HELPER_FIXTURE_ARTIFACT,
+    appBundleId: 'com.example.app',
+  }).then(
+    () => undefined,
+    (error: unknown) => error,
+  );
+  return { error, sessionCaptures };
+}
+
+test('a busy screen whose captures are cheap is re-captured before its verdict', async () => {
+  const { error, sessionCaptures } = await captureBusyScreen(10);
+
+  assert.ok(error instanceof AppError);
+  assert.equal(error.details?.attempts, 3);
+  assert.equal(sessionCaptures, 3);
+});
+
+test('a busy screen whose captures are slow is not re-captured past the window', async () => {
+  // One attempt that spends a whole helper command budget leaves no room in the daemon request
+  // envelope for two more of the same.
+  const { error, sessionCaptures } = await captureBusyScreen(30_000);
+
+  assert.ok(error instanceof AppError);
+  assert.equal(error.details?.attempts, 1);
+  assert.equal(sessionCaptures, 1);
+});
+
+test('a busy screen is not re-captured when the delay before it crosses the window', async () => {
+  const { error, sessionCaptures } = await captureBusyScreen(9_900);
+
+  assert.ok(error instanceof AppError);
+  assert.equal(error.details?.attempts, 1);
+  assert.equal(sessionCaptures, 1);
+});

packages/platform-android/src/__tests__/snapshot-helper-capture.test.ts

@@ -1,5 +1,7 @@
 import assert from 'node:assert/strict';
 import { beforeEach, test } from 'vitest';
+import { readAndroidCaptureFailureReason } from '@agent-device/contracts/android-snapshot-quality';
+import { AppError } from '@agent-device/kernel/errors';
 import { captureAndroidSnapshotWithHelper } from '../snapshot-helper-capture.ts';
 import { resetAndroidSnapshotHelperRetirements } from '../snapshot-helper-retirement.ts';
 import type { AndroidAdbExecutor } from '../snapshot-helper-types.ts';
@@ -130,6 +132,33 @@ test('canceled one-shot capture reports the cancellation and the next capture re
   ]);
 });
 
+test('a helper failure reported under a zero am exit status keeps its own reason', async () => {
+  // `am instrument` exits 0 after the helper finished with ok=false, as an emulator answered a
+  // capture of an app whose main thread never served its window.
+  const adb: AndroidAdbExecutor = async (args) => {
+    if (isAndroidHelperRuntimeProbe(args)) return androidHelperRuntimeProbeResult();
+    return {
+      exitCode: 0,
+      stdout: [
+        'INSTRUMENTATION_RESULT: agentDeviceProtocol=android-snapshot-helper-v1',
+        'INSTRUMENTATION_RESULT: errorType=java.util.concurrent.TimeoutException',
+        'INSTRUMENTATION_RESULT: message=Timed out waiting for the accessibility hierarchy',
+        'INSTRUMENTATION_RESULT: ok=false',
+        'INSTRUMENTATION_CODE: 1',
+      ].join('\n'),
+      stderr: '',
+    };
+  };
+
+  await assert.rejects(
+    captureAndroidSnapshotWithHelper({ adb, deviceKey: 'android:emulator-5554' }),
+    (error: unknown) =>
+      error instanceof AppError &&
+      readAndroidCaptureFailureReason(error) === 'accessibility-timeout' &&
+      error.details?.errorType === 'java.util.concurrent.TimeoutException',
+  );
+});
+
 function helperOutput(xml: string): string {
   return [
     'INSTRUMENTATION_STATUS: agentDeviceProtocol=android-snapshot-helper-v1',

packages/platform-android/src/__tests__/snapshot-helper-session-lifecycle.test.ts

@@ -137,27 +137,28 @@ test('a generous caller budget buys a slow start, and never more than the caller
 test('a session that reaches ready settles a release the device could not confirm', async () => {
   const calls: (readonly string[])[] = [];
   const spawnArgs: (readonly string[])[] = [];
+  const deviceKey = 'android:emulator-5554';
   // The device answers every process read with an adb error no classifier lists, and the first
-  // command's session stalls, so that command's teardown records a release nothing could prove.
+  // session ignores its quit, so that teardown records a release nothing could prove.
   const provider = createSessionProvider({
     calls,
     spawnArgs,
-    stalledSnapshots: 1,
+    quitResponseMode: 'malformed',
     runtimeRelease: 'closed',
   });
 
-  const stalled = await captureAndroidSnapshotWithHelperSession({
+  await captureAndroidSnapshotWithHelperSession({
     adb: provider.exec,
     adbProvider: provider,
-    deviceKey: 'android:emulator-5554',
+    deviceKey,
     commandTimeoutMs: 400,
   });
-  assert.equal(stalled, undefined);
+  await stopAndroidSnapshotHelperSession(deviceKey);
 
   const started = await captureAndroidSnapshotWithHelperSession({
     adb: provider.exec,
     adbProvider: provider,
-    deviceKey: 'android:emulator-5554',
+    deviceKey,
     commandTimeoutMs: 400,
   });
   assert.equal(started?.metadata.sessionReused, false);
@@ -169,7 +170,7 @@ test('a session that reaches ready settles a release the device could not confir
   const reused = await captureAndroidSnapshotWithHelperSession({
     adb: provider.exec,
     adbProvider: provider,
-    deviceKey: 'android:emulator-5554',
+    deviceKey,
     commandTimeoutMs: 400,
   });
 
@@ -181,6 +182,28 @@ test('a session that reaches ready settles a release the device could not confir
   );
 });
 
+test('a session whose capture timed out is not restarted by the next command', async () => {
+  const calls: (readonly string[])[] = [];
+  const spawnArgs: (readonly string[])[] = [];
+  const provider = createSessionProvider({ calls, spawnArgs, stalledSnapshots: 1 });
+  const capture = () =>
+    captureAndroidSnapshotWithHelperSession({
+      adb: provider.exec,
+      adbProvider: provider,
+      deviceKey: 'android:emulator-5554',
+      commandTimeoutMs: 400,
+    });
+
+  assert.equal(await capture(), undefined, 'the timed-out session falls back to one-shot');
+  assert.equal(await capture(), undefined, 'the next command answers one-shot as well');
+  assert.equal(spawnArgs.length, 1, 'a timed-out session earns a backoff, not another spawn');
+  assert.equal(
+    calls.some(isAndroidHelperRuntimeForceStop),
+    true,
+    'the timed-out helper is stopped',
+  );
+});
+
 test('starts and reuses a persistent Android snapshot helper session', async () => {
   const calls: (readonly string[])[] = [];
   const spawnArgs: (readonly string[])[] = [];

packages/platform-android/src/__tests__/snapshot-helper-session.test.ts

@@ -147,7 +147,8 @@ test('release the transport could not confirm falls back instead of failing the
     });
     assert.equal(output, undefined, `attempt ${attempt}`);
   }
-  assert.equal(processes.length, 2);
+  // The malformed answer backs the session off, so the second command goes straight to one-shot.
+  assert.equal(processes.length, 1);
 });
 
 test('capture refuses a device the previous teardown found the helper still running', async () => {

packages/platform-android/src/snapshot-helper-capture.ts

@@ -228,9 +228,13 @@ async function readFallbackHelperOutputOrThrow(
   result: Awaited<ReturnType<AndroidSnapshotHelperCaptureOptions['adb']>>,
   error: unknown,
 ): Promise<AndroidSnapshotHelperReadResult> {
-  if (error instanceof AppError && result.exitCode !== 0 && error.details?.helper) throw error;
+  const helperFailure = error instanceof AppError && error.details?.helper ? error : undefined;
+  if (helperFailure && result.exitCode !== 0) throw helperFailure;
   const fileOutput = await readFallbackHelperOutputFile(options, resolved, result);
   if (fileOutput) return { output: fileOutput, cleanupDone: true };
+  // `am instrument` exits 0 after a helper that reported its own failure, so that report is the
+  // answer whenever no output file stands in for it.
+  if (helperFailure) throw helperFailure;
   // exec-guard-allow: reachable at exit 0 (helper output unparseable); the
   // message already branches on the exit code.
   throw new AppError(

packages/platform-android/src/snapshot-helper-session-lifecycle.ts

@@ -58,13 +58,13 @@ const SESSION_REQUEST_OVERHEAD_MS = 3_000;
 const FORWARD_TIMEOUT_MS = 5_000;
 // A helper that cannot start spends its whole start budget failing, and the one-shot transport that
 // answers afterwards still has to run. Retrying that on the very next command is what made commands
-// on the slow hosts of #2553 take roughly twice as long, so a failed start keeps the persistent path
-// away for at least this long.
-const SESSION_START_RETRY_FLOOR_MS = 10_000;
-// …and for no longer than this, however long the start took. The floor keeps a burst of commands
+// on the slow hosts of #2553 take roughly twice as long, so a failed start or session capture keeps
+// the persistent path away for at least this long.
+const SESSION_RETRY_FLOOR_MS = 10_000;
+// …and for no longer than this, however long the failure took. The floor keeps a burst of commands
 // from re-paying an instant failure; the ceiling keeps a host whose helper is simply broken from
 // being written off for longer than a working session would have lasted.
-const SESSION_START_RETRY_CEILING_MS = 60_000;
+const SESSION_RETRY_CEILING_MS = 60_000;
 
 export type AndroidSnapshotHelperSessionHelperIdentity = {
   packageName: string;
@@ -93,8 +93,11 @@ export type AndroidSnapshotHelperSessionAcquisition = {
 };
 
 const sessions = new Map<string, AndroidSnapshotHelperSession>();
-/** Capture identity → when this process may spawn that helper build again after a failed start. */
-const failedStarts = new Map<string, number>();
+/**
+ * Capture identity → when this process may spawn that helper build again after a failed start or a
+ * failed session capture.
+ */
+const failedSessions = new Map<string, number>();
 
 /**
  * Starts (or reuses) the session without capturing, so a helper-backed read that is not a snapshot
@@ -150,7 +153,7 @@ async function resolveAndroidSnapshotHelperSession(params: {
   resolved: AndroidSnapshotHelperResolvedCaptureOptions;
   startBudgetMs: number;
 }): Promise<AndroidSnapshotHelperSession | undefined> {
-  if (isAndroidSnapshotHelperStartBackedOff(params.identity)) return undefined;
+  if (isAndroidSnapshotHelperSessionBackedOff(params.identity)) return undefined;
   await retireUnusableAndroidSnapshotHelperSession(params.deviceKey, params.identity);
   return sessions.get(params.deviceKey) ?? (await tryStartAndroidSnapshotHelperSession(params));
 }
@@ -186,9 +189,9 @@ async function tryStartAndroidSnapshotHelperSession(params: {
     return await startAndroidSnapshotHelperSession(params);
   } catch (error) {
     params.options.signal?.throwIfAborted();
-    failedStarts.set(
+    failedSessions.set(
       params.identity,
-      Date.now() + androidSnapshotHelperStartRetryAfterMs(Date.now() - startedAtMs),
+      Date.now() + androidSnapshotHelperSessionRetryAfterMs(Date.now() - startedAtMs),
     );
     emitDiagnostic({
       level: 'warn',
@@ -203,24 +206,53 @@ async function tryStartAndroidSnapshotHelperSession(params: {
   }
 }
 
-/** A helper build whose last start failed is left alone until that start's backoff has run out. */
-function isAndroidSnapshotHelperStartBackedOff(identity: string): boolean {
-  const retryAtMs = failedStarts.get(identity);
+/** A helper build whose last start or capture failed is left alone until its backoff has run out. */
+function isAndroidSnapshotHelperSessionBackedOff(identity: string): boolean {
+  const retryAtMs = failedSessions.get(identity);
   if (retryAtMs === undefined) return false;
   if (retryAtMs > Date.now()) return true;
-  failedStarts.delete(identity);
+  failedSessions.delete(identity);
   return false;
 }
 
 /**
- * How long a failed start earns: as long as it spent failing, because a start that burned half a
- * minute on a wedged device would burn another half minute on the next command, bounded so a burst
+ * How long a failed start or capture earns: as long as it spent failing, because a start that burned
+ * half a minute on a wedged device would burn another half minute on the next command, bounded so a burst
  * of commands neither re-pays an instant failure nor writes a device off for the rest of the run.
  */
-function androidSnapshotHelperStartRetryAfterMs(startDurationMs: number): number {
-  return Math.min(
-    Math.max(startDurationMs, SESSION_START_RETRY_FLOOR_MS),
-    SESSION_START_RETRY_CEILING_MS,
+function androidSnapshotHelperSessionRetryAfterMs(failureDurationMs: number): number {
+  return Math.min(Math.max(failureDurationMs, SESSION_RETRY_FLOOR_MS), SESSION_RETRY_CEILING_MS);
+}
+
+/**
+ * Tears down a session whose capture failed and keeps its helper build off the persistent path for
+ * as long as a failed start would, counted from when the one-shot capture that answers instead has
+ * had its whole budget. A session that timed out once times out again on the same screen, so
+ * restarting it on the next attempt or command only spends its budget again before the one-shot
+ * transport answers. A cancellation is the caller's deadline, not evidence about the helper, so it
+ * earns no backoff.
+ */
+export async function retireFailedAndroidSnapshotHelperSession(params: {
+  deviceKey: string;
+  /** Helper build of the session whose capture failed. */
+  identity: string;
+  failedAfterMs: number;
+  /** Budget of the one-shot capture that answers in place of the failed session. */
+  fallbackBudgetMs: number;
+  signal?: AbortSignal;
+  cause: unknown;
+}): Promise<void> {
+  await stopAndroidSnapshotHelperSession(params.deviceKey, {
+    force: true,
+    signal: params.signal,
+    cause: params.cause,
+  });
+  if (params.signal?.aborted) return;
+  failedSessions.set(
+    params.identity,
+    Date.now() +
+      params.fallbackBudgetMs +
+      androidSnapshotHelperSessionRetryAfterMs(params.failedAfterMs),
   );
 }
 
@@ -285,7 +317,7 @@ async function startAndroidSnapshotHelperSession(params: {
       params.options.signal,
     );
     sessions.set(params.deviceKey, session);
-    failedStarts.delete(params.identity);
+    failedSessions.delete(params.identity);
     // `am instrument` force-stops whatever is already instrumenting this package, so a helper that
     // reported itself ready is the only helper process the device has left, and the release the
     // previous teardown could not prove went away with the process that owed it. Leaving the entry
@@ -530,7 +562,7 @@ export async function resetAndroidSnapshotHelperSessions(): Promise<void> {
     // One teardown that throws must not leave the next caller believing a session, a pending
     // retirement, or a failed start is still standing.
     sessions.clear();
-    failedStarts.clear();
+    failedSessions.clear();
     resetAndroidSnapshotHelperRetirements();
     resetAndroidAdbShellProtocolProbes();
   }