refactor(android): enforce host-adb transport failure contract

Author: thymikee

src/platforms/android/__tests__/adb-host-transport.test.ts

@@ -2,8 +2,6 @@ import assert from 'node:assert/strict';
 import { test } from 'vitest';
 import { AppError } from '@agent-device/kernel/errors';
 import { runAndroidHostAdb, withAndroidHostAdbTransport } from '../adb-host-transport.ts';
-import { listAndroidAdbSerialsQuick } from '../ime-lifecycle.ts';
-import { createLimrunRuntimeDependencies } from '../../../sdk/limrun-runtime-dependencies.ts';
 import {
   withCommandExecutorOverride,
   type CommandExecutorOverride,
@@ -70,10 +68,12 @@ test('without a transport host adb falls back to local runCmd with argv and opti
 });
 
 test('transport results and errors pass through with their ExecResult shape', async () => {
+  // Nonzero results only pass through under allowFailure; the throw contract
+  // has its own tests below.
   const scripted: ExecResult = { stdout: '', stderr: 'boom', exitCode: 7 };
   const result = await withAndroidHostAdbTransport(
     async () => scripted,
-    async () => await runAndroidHostAdb(['devices']),
+    async () => await runAndroidHostAdb(['devices'], { allowFailure: true }),
   );
   assert.deepEqual(result, scripted);
 
@@ -82,7 +82,7 @@ test('transport results and errors pass through with their ExecResult shape', as
   const sloppy = { stdout: undefined, stderr: null, exitCode: '1' } as unknown as ExecResult;
   const coerced = await withAndroidHostAdbTransport(
     async () => sloppy,
-    async () => await runAndroidHostAdb(['devices']),
+    async () => await runAndroidHostAdb(['devices'], { allowFailure: true }),
   );
   assert.deepEqual(coerced, { stdout: '', stderr: '', exitCode: 1 });
 
@@ -98,6 +98,36 @@ test('transport results and errors pass through with their ExecResult shape', as
   );
 });
 
+test('a transport nonzero exit without allowFailure throws the classified adb failure', async () => {
+  const error = await withAndroidHostAdbTransport(
+    async () => ({ stdout: '', stderr: 'error: device offline', exitCode: 1 }),
+    async () =>
+      await runAndroidHostAdb(['devices']).then(
+        () => assert.fail('expected the host adb call to reject'),
+        (err: unknown) => err,
+      ),
+  );
+
+  assert.ok(error instanceof AppError);
+  assert.equal(error.code, 'COMMAND_FAILED');
+  assert.equal(error.message, 'adb devices exited with code 1');
+  assert.equal(error.details?.adbFailure, 'device_offline');
+  assert.equal(error.details?.retriable, true);
+  assert.match(String(error.details?.hint), /adb reconnect/i);
+  assert.equal(error.details?.stderr, 'error: device offline');
+});
+
+test('a transport nonzero exit with allowFailure returns the result untouched', async () => {
+  const scripted: ExecResult = { stdout: '', stderr: 'error: device offline', exitCode: 1 };
+
+  const result = await withAndroidHostAdbTransport(
+    async () => scripted,
+    async () => await runAndroidHostAdb(['devices'], { allowFailure: true }),
+  );
+
+  assert.deepEqual(result, scripted);
+});
+
 test('nested transport scopes compose innermost-wins and restore on scope end', async () => {
   const calls: string[] = [];
   const transportFor = (name: string) => async () => {
@@ -120,44 +150,3 @@ test('nested transport scopes compose innermost-wins and restore on scope end',
   assert.deepEqual(calls, ['outer', 'inner', 'outer']);
   assert.equal(driver.calls.length, 1);
 });
-
-test('listAndroidAdbSerialsQuick routes its global devices call through the transport', async () => {
-  const seenArgs: string[][] = [];
-
-  const serials = await withAndroidHostAdbTransport(
-    async (args) => {
-      seenArgs.push([...args]);
-      return {
-        stdout: 'List of devices attached\nemulator-5554\tdevice\n',
-        stderr: '',
-        exitCode: 0,
-      };
-    },
-    async () => await listAndroidAdbSerialsQuick(),
-  );
-
-  assert.deepEqual(serials, ['emulator-5554']);
-  assert.deepEqual(seenArgs, [['devices']]);
-});
-
-test('limrun host.runAdb keeps its exported shape and routes through the transport', async () => {
-  const dependencies = createLimrunRuntimeDependencies();
-  const seen: Array<{ args: string[]; options?: ExecOptions }> = [];
-
-  const result = await withAndroidHostAdbTransport(
-    async (args, options) => {
-      seen.push({ args, ...(options ? { options } : {}) });
-      return { stdout: 'ok', stderr: '', exitCode: 0 };
-    },
-    async () =>
-      await dependencies.host.runAdb(['disconnect', 'emulator-5554'], {
-        allowFailure: true,
-        timeoutMs: 10_000,
-      }),
-  );
-
-  assert.deepEqual(result, { stdout: 'ok', stderr: '', exitCode: 0 });
-  assert.deepEqual(seen, [
-    { args: ['disconnect', 'emulator-5554'], options: { allowFailure: true, timeoutMs: 10_000 } },
-  ]);
-});

src/platforms/android/__tests__/ime-lifecycle.test.ts

@@ -37,10 +37,12 @@ vi.mock('../ime-helper.ts', async (importOriginal) => {
 
 import { ANDROID_EMULATOR } from '../../../__tests__/test-utils/device-fixtures.ts';
 import { withAndroidAdbProvider, type AndroidAdbExecutor } from '../adb-executor.ts';
+import { withAndroidHostAdbTransport } from '../adb-host-transport.ts';
 import { resetAndroidImeHelperInstallCache } from '../ime-helper.ts';
 import {
   activateAndroidTestIme,
   isAndroidTestImeActive,
+  listAndroidAdbSerialsQuick,
   restoreAndroidTestIme,
   restoreOrphanedAndroidTestImeOnDaemonStartup,
   resetAndroidTestImeActivationCacheForTests,
@@ -492,3 +494,33 @@ test('startup recovery tolerates a serial listing failure and keeps the marker',
     assert.equal(await pendingMarkerExists(stateDir, SERIAL), true);
   });
 });
+
+test('listAndroidAdbSerialsQuick routes its global devices call through the host transport', async () => {
+  const seenArgs: string[][] = [];
+
+  const serials = await withAndroidHostAdbTransport(
+    async (args) => {
+      seenArgs.push([...args]);
+      return {
+        stdout: 'List of devices attached\nemulator-5554\tdevice\n',
+        stderr: '',
+        exitCode: 0,
+      };
+    },
+    async () => await listAndroidAdbSerialsQuick(),
+  );
+
+  assert.deepEqual(serials, ['emulator-5554']);
+  assert.deepEqual(seenArgs, [['devices']]);
+});
+
+test('listAndroidAdbSerialsQuick yields no serials when the transport exits nonzero', async () => {
+  const serials = await withAndroidHostAdbTransport(
+    async () => ({ stdout: '', stderr: 'error: device offline', exitCode: 1 }),
+    async () => await listAndroidAdbSerialsQuick(),
+  );
+
+  // The seam throws on nonzero (no allowFailure here); the quick listing's
+  // catch treats any failure as an empty inventory.
+  assert.deepEqual(serials, []);
+});

src/platforms/android/adb-host-transport.ts

@@ -1,10 +1,22 @@
 import { AsyncLocalStorage } from 'node:async_hooks';
 import { coerceExecResult, runCmd, type ExecOptions, type ExecResult } from '../../utils/exec.ts';
+import { androidAdbResultError } from './adb-executor.ts';
 
 /**
  * Runs one host-level adb invocation — global (`adb devices`) or device-scoped
  * (`adb -s <serial> …`). Providers install a scoped transport so host-level adb
  * traffic follows their tunnel instead of shelling out to local adb.
+ *
+ * Precedence: `runAndroidHostAdb` consults an installed transport BEFORE the
+ * exec layer, so a transport wins over any surrounding AndroidAdbProvider
+ * command-executor override — including `-s <serial>` argv belonging to other
+ * devices and global commands. Installers must therefore install outside
+ * request-bound provider scopes, or filter serials inside the implementation.
+ *
+ * Implementations receive the raw argv exactly as the host would run it and are
+ * held to the same allowFailure/throw-on-nonzero contract the local fallback
+ * provides: the seam enforces it by throwing the classified adb-failure error
+ * on a nonzero exit unless `options.allowFailure` is set.
  */
 export type AndroidAdbHostTransport = (
   args: string[],
@@ -15,14 +27,25 @@ const androidAdbHostTransportScope = new AsyncLocalStorage<AndroidAdbHostTranspo
 
 // Single construction path for host adb. Without an installed transport this
 // must stay plain `runCmd('adb', …)` so the exec layer's command-executor
-// interception (provider scopes) sees the call exactly as before.
+// interception (provider scopes) sees the call exactly as before. Both arms
+// share one failure contract: a nonzero exit rejects with the classified adb
+// error unless `allowFailure` is set, so consumers cannot observe divergent
+// semantics between an installed transport and local execution.
 export async function runAndroidHostAdb(
   args: string[],
   options?: ExecOptions,
 ): Promise<ExecResult> {
   const transport = androidAdbHostTransportScope.getStore();
-  if (!transport) return await runCmd('adb', args, options);
-  return coerceExecResult(await transport(args, options));
+  const result = transport
+    ? coerceExecResult(await transport(args, options))
+    : await runCmd('adb', args, options);
+  if (!options?.allowFailure && result.exitCode !== 0) {
+    throw androidAdbResultError(
+      `adb ${args.join(' ')} exited with code ${result.exitCode}`,
+      result,
+    );
+  }
+  return result;
 }
 
 export async function withAndroidHostAdbTransport<T>(

src/sdk/limrun-runtime-dependencies.test.ts

@@ -136,3 +136,29 @@ test('Limrun appstate forwards an in-flight abort through the provider ADB execu
   await expect(pending).rejects.toMatchObject({ name: 'AbortError' });
   assert.equal(observedSignal, controller.signal);
 });
+
+test('host.runAdb keeps its exported shape and routes through the host transport', async () => {
+  const { createLimrunRuntimeDependencies } = await import('./limrun-runtime-dependencies.ts');
+  const { withAndroidHostAdbTransport } = await import(
+    '../platforms/android/adb-host-transport.ts'
+  );
+  const dependencies = createLimrunRuntimeDependencies();
+  const seen: Array<{ args: string[]; options?: Record<string, unknown> }> = [];
+
+  const result = await withAndroidHostAdbTransport(
+    async (args, options) => {
+      seen.push({ args, ...(options ? { options } : {}) });
+      return { stdout: 'ok', stderr: '', exitCode: 0 };
+    },
+    async () =>
+      await dependencies.host.runAdb(['disconnect', 'emulator-5554'], {
+        allowFailure: true,
+        timeoutMs: 10_000,
+      }),
+  );
+
+  assert.deepEqual(result, { stdout: 'ok', stderr: '', exitCode: 0 });
+  assert.deepEqual(seen, [
+    { args: ['disconnect', 'emulator-5554'], options: { allowFailure: true, timeoutMs: 10_000 } },
+  ]);
+});