Skip to content

Commit fe1e7c0

Browse files
authored
refactor(android): one injectable host-adb transport for the remaining raw adb sites (#2052)
* refactor(android): one injectable host-adb transport * refactor(android): enforce host-adb transport failure contract * style(android): format relocated limrun transport test * refactor(android): colocate host adb transport * refactor(android): route host adb through composition * fix(android): keep unbound adb host loud
1 parent adcf263 commit fe1e7c0

11 files changed

Lines changed: 268 additions & 16 deletions
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import { AppError } from '@agent-device/kernel/errors';
4+
import { bindAndroidAdbHostStub } from './adb-host.fixtures.ts';
5+
import { runAndroidHostAdb, withAndroidHostAdbTransport } from './adb-executor.ts';
6+
7+
test('a scoped transport intercepts host adb without reaching the injected host', async () => {
8+
let hostCalls = 0;
9+
bindAndroidAdbHostStub({
10+
execHostAdb: async () => {
11+
hostCalls += 1;
12+
return { stdout: 'host', stderr: '', exitCode: 0 };
13+
},
14+
});
15+
16+
const result = await withAndroidHostAdbTransport(
17+
async (args, options) => {
18+
assert.deepEqual(args, ['devices']);
19+
assert.deepEqual(options, { timeoutMs: 1_234 });
20+
return { stdout: 'transport', stderr: '', exitCode: 0 };
21+
},
22+
async () => await runAndroidHostAdb(['devices'], { timeoutMs: 1_234 }),
23+
);
24+
25+
assert.equal(result.stdout, 'transport');
26+
assert.equal(hostCalls, 0);
27+
});
28+
29+
test('the local host arm always obtains a result before applying the shared failure contract', async () => {
30+
let receivedOptions: Record<string, unknown> | undefined;
31+
bindAndroidAdbHostStub({
32+
execHostAdb: async (_args, options) => {
33+
receivedOptions = options;
34+
return { stdout: '', stderr: 'error: device offline', exitCode: 1 };
35+
},
36+
});
37+
38+
const error = await runAndroidHostAdb(['devices']).then(
39+
() => assert.fail('expected the host adb call to reject'),
40+
(cause: unknown) => cause,
41+
);
42+
43+
assert.deepEqual(receivedOptions, { allowFailure: true });
44+
assert.ok(error instanceof AppError);
45+
assert.equal(error.details?.adbFailure, 'device_offline');
46+
assert.equal(error.details?.retriable, true);
47+
assert.match(String(error.details?.hint), /adb reconnect/i);
48+
});
49+
50+
test('allowFailure returns a nonzero local result unchanged', async () => {
51+
const scripted = { stdout: '', stderr: 'offline', exitCode: 7 };
52+
bindAndroidAdbHostStub({ execHostAdb: async () => scripted });
53+
54+
assert.deepEqual(
55+
await runAndroidHostAdb(['devices'], { allowFailure: true, timeoutMs: 5_000 }),
56+
scripted,
57+
);
58+
});
59+
60+
test('unchecked transport results are normalized at the package boundary', async () => {
61+
bindAndroidAdbHostStub({
62+
coerceAdbResult: (result) => ({
63+
...result,
64+
stdout: typeof result.stdout === 'string' ? result.stdout : '',
65+
stderr: typeof result.stderr === 'string' ? result.stderr : '',
66+
exitCode: Number(result.exitCode),
67+
}),
68+
});
69+
const sloppy = { stdout: undefined, stderr: null, exitCode: '1' } as never;
70+
71+
const result = await withAndroidHostAdbTransport(
72+
async () => sloppy,
73+
async () => await runAndroidHostAdb(['devices'], { allowFailure: true }),
74+
);
75+
76+
assert.deepEqual(result, { stdout: '', stderr: '', exitCode: 1 });
77+
});
78+
79+
test('nested transport scopes are innermost-first and restore on scope exit', async () => {
80+
bindAndroidAdbHostStub({
81+
execHostAdb: async () => ({ stdout: 'host', stderr: '', exitCode: 0 }),
82+
});
83+
const transportFor = (name: string) => async () => ({
84+
stdout: name,
85+
stderr: '',
86+
exitCode: 0,
87+
});
88+
89+
await withAndroidHostAdbTransport(transportFor('outer'), async () => {
90+
assert.equal((await runAndroidHostAdb(['devices'])).stdout, 'outer');
91+
await withAndroidHostAdbTransport(transportFor('inner'), async () => {
92+
assert.equal((await runAndroidHostAdb(['devices'])).stdout, 'inner');
93+
});
94+
assert.equal((await runAndroidHostAdb(['devices'])).stdout, 'outer');
95+
});
96+
assert.equal((await runAndroidHostAdb(['devices'])).stdout, 'host');
97+
});

packages/platform-android/src/adb-executor.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export {
1010
attachAdbFailureHint,
1111
classifyAndroidAdbFailure as classifyAdbFailure,
1212
} from './adb-failure.ts';
13+
export { runAndroidHostAdb, withAndroidHostAdbTransport } from './adb-host.ts';
1314
export { createAndroidPortReverseManager } from './adb-port-reverse.ts';
1415
export {
1516
createDeviceAdbExecutor,

packages/platform-android/src/adb-failure.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ export function classifyAndroidAdbFailure(
134134
import { AppError } from '@agent-device/kernel/errors';
135135
import type { HostCommandResult } from '@agent-device/contracts/platform-runtime-host';
136136
import type { AndroidAdbExecutorResult } from './adb-transport.ts';
137-
import { requireAndroidAdbHost } from './adb-host.ts';
138137

139138
/**
140139
* Enriches a failed adb command error in place with the classified hint,
@@ -178,12 +177,11 @@ function classifyAdbCommandError(error: AppError): AndroidAdbFailureClassificati
178177
* Site-provided `details` win on key collisions, and a site `hint` is preserved
179178
* over the classified one.
180179
*
181-
* Nonzero exits build their details via the host's execFailureDetails, whose
182-
* processExitError flag makes normalizeError append the first stderr line to
183-
* the curated message — the classified hint and the stderr-excerpt enrichment
184-
* compose instead of competing. Semantic failures thrown at exit 0 (e.g. an
185-
* `am start` error printed on a successful exit) stay unflagged so a stray
186-
* stderr line never decorates a message the process exit does not back up.
180+
* Nonzero exits set processExitError so normalizeError appends the first stderr
181+
* line to the curated message — the classified hint and stderr-excerpt
182+
* enrichment compose instead of competing. Semantic failures thrown at exit 0
183+
* (e.g. an `am start` error printed on a successful exit) stay unflagged so a
184+
* stray stderr line never decorates a message the process exit does not back up.
187185
*/
188186
export function androidAdbResultError(
189187
message: string,
@@ -193,7 +191,13 @@ export function androidAdbResultError(
193191
const failureDetails =
194192
result.exitCode === 0
195193
? { stdout: result.stdout, stderr: result.stderr, exitCode: result.exitCode, ...details }
196-
: requireAndroidAdbHost().execFailureDetails(result, details);
194+
: {
195+
stdout: result.stdout,
196+
stderr: result.stderr,
197+
exitCode: result.exitCode,
198+
processExitError: true,
199+
...details,
200+
};
197201
return attachAdbFailureHint(new AppError('COMMAND_FAILED', message, failureDetails));
198202
}
199203

packages/platform-android/src/adb-host.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { AsyncLocalStorage } from 'node:async_hooks';
12
import type {
23
AndroidHelperInstallDecision,
34
AndroidImeHelperArtifact,
@@ -95,6 +96,14 @@ export type AndroidAdbHost = Readonly<{
9596

9697
let boundHost: AndroidAdbHost | undefined;
9798

99+
/** Scoped override for host-global and explicitly serial-qualified adb argv. */
100+
export type AndroidAdbHostTransport = (
101+
args: string[],
102+
options?: AndroidAdbExecutorOptions,
103+
) => Promise<AndroidAdbExecutorResult>;
104+
105+
const androidAdbHostTransportScope = new AsyncLocalStorage<AndroidAdbHostTransport>();
106+
98107
/** Composition-time wiring; the last bind wins so test harnesses can rebind. */
99108
export function bindAndroidAdbHost(host: AndroidAdbHost): void {
100109
boundHost = host;
@@ -110,6 +119,39 @@ export function requireAndroidAdbHost(): AndroidAdbHost {
110119
return boundHost;
111120
}
112121

122+
/**
123+
* Runs host-level adb through one package-owned failure contract. A scoped
124+
* transport wins over the injected local host port; nested scopes are
125+
* innermost-first and restore automatically.
126+
*/
127+
export async function runAndroidHostAdb(
128+
args: string[],
129+
options?: AndroidAdbExecutorOptions,
130+
): Promise<AndroidAdbExecutorResult> {
131+
const host = requireAndroidAdbHost();
132+
const transport = androidAdbHostTransportScope.getStore();
133+
const result = host.coerceAdbResult(
134+
transport
135+
? await transport(args, options)
136+
: await host.execHostAdb(args, { ...options, allowFailure: true }),
137+
);
138+
if (!options?.allowFailure && result.exitCode !== 0) {
139+
const { androidAdbResultError } = await import('./adb-failure.ts');
140+
throw androidAdbResultError(
141+
`adb ${args.join(' ')} exited with code ${result.exitCode}`,
142+
result,
143+
);
144+
}
145+
return result;
146+
}
147+
148+
export async function withAndroidHostAdbTransport<T>(
149+
transport: AndroidAdbHostTransport,
150+
fn: () => Promise<T>,
151+
): Promise<T> {
152+
return await androidAdbHostTransportScope.run(transport, fn);
153+
}
154+
113155
export function emitAndroidAdbDiagnostic(event: AndroidAdbDiagnosticEvent): void {
114156
requireAndroidAdbHost().emitDiagnostic(event);
115157
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import { bindAndroidAdbHostStub } from './adb-host.fixtures.ts';
4+
import { withAndroidHostAdbTransport } from './adb-executor.ts';
5+
import { listAndroidAdbSerialsQuick } from './ime-lifecycle.ts';
6+
7+
test('quick serial listing keeps an unbound adb host port loud', async () => {
8+
await assert.rejects(
9+
async () => await listAndroidAdbSerialsQuick(),
10+
/Android adb host port is not bound/,
11+
);
12+
});
13+
14+
test('quick serial listing routes its global devices call through the scoped transport', async () => {
15+
bindAndroidAdbHostStub();
16+
const seenArgs: string[][] = [];
17+
18+
const serials = await withAndroidHostAdbTransport(
19+
async (args) => {
20+
seenArgs.push([...args]);
21+
return {
22+
stdout: 'List of devices attached\nemulator-5554\tdevice\n',
23+
stderr: '',
24+
exitCode: 0,
25+
};
26+
},
27+
async () => await listAndroidAdbSerialsQuick(),
28+
);
29+
30+
assert.deepEqual(serials, ['emulator-5554']);
31+
assert.deepEqual(seenArgs, [['devices']]);
32+
});
33+
34+
test('quick serial listing treats a classified transport failure as empty inventory', async () => {
35+
bindAndroidAdbHostStub();
36+
const serials = await withAndroidHostAdbTransport(
37+
async () => ({ stdout: '', stderr: 'error: device offline', exitCode: 1 }),
38+
async () => await listAndroidAdbSerialsQuick(),
39+
);
40+
41+
assert.deepEqual(serials, []);
42+
});

packages/platform-android/src/ime-lifecycle.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// `ime-activation.ts` (the activation transaction), and `ime-restore.ts` (restore + startup
55
// orphan recovery).
66

7-
import { requireAndroidAdbHost } from './adb-host.ts';
7+
import { requireAndroidAdbHost, runAndroidHostAdb } from './adb-host.ts';
88

99
export { activateAndroidTestIme } from './ime-activation.ts';
1010
export {
@@ -23,11 +23,9 @@ export {
2323

2424
// Serials only, with no full-inventory name, boot-state, or target probes.
2525
export async function listAndroidAdbSerialsQuick(): Promise<string[]> {
26-
// Resolved outside the try: an unbound host port is a wiring bug and must stay loud, while a
27-
// failed adb execution legitimately reads as "no devices".
28-
const host = requireAndroidAdbHost();
26+
requireAndroidAdbHost();
2927
try {
30-
const result = await host.execHostAdb(['devices'], { timeoutMs: 5_000 });
28+
const result = await runAndroidHostAdb(['devices'], { timeoutMs: 5_000 });
3129
return result.stdout
3230
.split('\n')
3331
.map((line) => line.trim())
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import assert from 'node:assert/strict';
2+
import fs from 'node:fs';
3+
import path from 'node:path';
4+
import { test } from 'vitest';
5+
import { AppError } from '@agent-device/kernel/errors';
6+
import { runAndroidHostAdb } from '@agent-device/platform-android/adb-executor';
7+
import { mkdtempForTestSync } from '../../../__tests__/test-utils/tmp-dir.ts';
8+
import '../adb-host-binding.ts';
9+
10+
test.skipIf(process.platform === 'win32')(
11+
'the local host binding classifies a real nonzero adb process result',
12+
async () => {
13+
const tmpDir = mkdtempForTestSync('agent-device-adb-host-binding-');
14+
const adbPath = path.join(tmpDir, 'adb');
15+
fs.writeFileSync(
16+
adbPath,
17+
'#!/usr/bin/env node\nprocess.stderr.write("error: device offline\\n"); process.exit(1);',
18+
);
19+
fs.chmodSync(adbPath, 0o755);
20+
const previousPath = process.env.PATH;
21+
process.env.PATH = `${tmpDir}${path.delimiter}${previousPath ?? ''}`;
22+
try {
23+
const error = await runAndroidHostAdb(['devices']).then(
24+
() => assert.fail('expected local adb to reject'),
25+
(cause: unknown) => cause,
26+
);
27+
28+
assert.ok(error instanceof AppError);
29+
assert.equal(error.details?.adbFailure, 'device_offline');
30+
assert.equal(error.details?.retriable, true);
31+
assert.match(String(error.details?.hint), /adb reconnect/i);
32+
} finally {
33+
if (previousPath === undefined) {
34+
delete process.env.PATH;
35+
} else {
36+
process.env.PATH = previousPath;
37+
}
38+
}
39+
},
40+
);

src/platforms/android/adb-executor.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ export {
1919
resolveAndroidTextInjector,
2020
resolveAndroidTouchProvider,
2121
resolveScopedAndroidAdbBackgroundTransport,
22+
runAndroidHostAdb,
2223
withAndroidAdbProvider,
24+
withAndroidHostAdbTransport,
2325
type AndroidAdbExecutor,
2426
type AndroidAdbExecutorOptions,
2527
type AndroidAdbExecutorResult,

src/platforms/android/emulator-lifecycle.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import type { DeviceInventoryRequest } from '@agent-device/contracts/device';
22
import type { DeviceInfo } from '@agent-device/kernel/device';
33
import { AppError, asAppError } from '@agent-device/kernel/errors';
44
import type { ExecResult } from '../../utils/exec.ts';
5-
import { runCmd, runCmdDetached, whichCmd } from '../../utils/exec.ts';
5+
import { runCmdDetached, whichCmd } from '../../utils/exec.ts';
6+
import { runAndroidHostAdb } from './adb-executor.ts';
67
import { Deadline, retryWithPolicy } from '../../utils/retry.ts';
78
import { sleep } from '../../utils/timeouts.ts';
89
import { bootFailureHint, classifyBootFailure } from '../boot-diagnostics.ts';
@@ -154,7 +155,7 @@ async function readAndroidBootProp(
154155
timeoutMs = ANDROID_BOOT_PROP_TIMEOUT_MS,
155156
signal?: AbortSignal,
156157
): Promise<ExecResult> {
157-
return await runCmd('adb', ['-s', serial, 'shell', 'getprop', 'sys.boot_completed'], {
158+
return await runAndroidHostAdb(['-s', serial, 'shell', 'getprop', 'sys.boot_completed'], {
158159
allowFailure: true,
159160
signal,
160161
timeoutMs,

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,27 @@ test('Limrun appstate forwards an in-flight abort through the provider ADB execu
136136
await expect(pending).rejects.toMatchObject({ name: 'AbortError' });
137137
assert.equal(observedSignal, controller.signal);
138138
});
139+
140+
test('host.runAdb keeps its exported shape and routes through the host transport', async () => {
141+
const { createLimrunRuntimeDependencies } = await import('./limrun-runtime-dependencies.ts');
142+
const { withAndroidHostAdbTransport } = await import('../platforms/android/adb-executor.ts');
143+
const dependencies = createLimrunRuntimeDependencies();
144+
const seen: Array<{ args: string[]; options?: Record<string, unknown> }> = [];
145+
146+
const result = await withAndroidHostAdbTransport(
147+
async (args, options) => {
148+
seen.push({ args, ...(options ? { options } : {}) });
149+
return { stdout: 'ok', stderr: '', exitCode: 0 };
150+
},
151+
async () =>
152+
await dependencies.host.runAdb(['disconnect', 'emulator-5554'], {
153+
allowFailure: true,
154+
timeoutMs: 10_000,
155+
}),
156+
);
157+
158+
assert.deepEqual(result, { stdout: 'ok', stderr: '', exitCode: 0 });
159+
assert.deepEqual(seen, [
160+
{ args: ['disconnect', 'emulator-5554'], options: { allowFailure: true, timeoutMs: 10_000 } },
161+
]);
162+
});

0 commit comments

Comments
 (0)