diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f7097fd1e..60fd6dcf94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,16 @@ shows that sheet (hosted out of the app process)." to "This snapshot shows a system web sign-in sheet presented over the app (hosted out of the app process), not app content". The payment host says "the system Apple Pay sheet" instead. +- Changed (android): a private adb server no longer absorbs a port its caller named. An adb request + that names a server other than the one its route holds — `-P 5037` in argv, or `serverPort` in the + call's options — is now refused with `managed-device-transport-mismatch` before anything is + dispatched, where previously a `-P` in argv was rewritten onto the route's own port. A caller that + asked for 5037 could read a zero exit as an answer about 5037 after the request had run on 15038. + It reaches `createLocalAndroidAdbProvider(device, { serverPort })`, + `runAndroidHostAdb(invocation, { serverPort })`, and the Limrun runtime dependency's adb calls. A + request that names no server, or names the one the route already holds, runs exactly as before, and + ambient adb is unchanged: with no private server named, the caller's argv is what runs, `-P` + included (#2632). - Fixed (android): a chunked `record stop` (recordings over 170 s) no longer warns that screenrecord stopped before record stop at the 180 s limit. Rotation always ends every earlier chunk before stop, so the warning now fires only when the last chunk's recorder had already exited. diff --git a/packages/platform-android/src/__tests__/adb-executor.test.ts b/packages/platform-android/src/__tests__/adb-executor.test.ts index f8a3eb69cb..57e9005162 100644 --- a/packages/platform-android/src/__tests__/adb-executor.test.ts +++ b/packages/platform-android/src/__tests__/adb-executor.test.ts @@ -245,15 +245,17 @@ test('createLocalAndroidAdbProvider carries a private server port through every await provider.pull?.('/sdcard/video.mp4', '/tmp/video.mp4'); await provider.install?.('/tmp/app.apk'); - assert.equal(readServerPort(mockRunCmdBackground.mock.calls[0]?.[2]), 15_037); + assert.equal(readServerPortArgv(mockRunCmdBackground.mock.calls[0]?.[1]), 15_037); assert.equal(mockRunCmd.mock.calls.length, 4); - for (const call of mockRunCmd.mock.calls) assert.equal(readServerPort(call[2]), 15_037); + for (const call of mockRunCmd.mock.calls) assert.equal(readServerPortArgv(call[1]), 15_037); }); -function readServerPort(options: unknown): number | undefined { - if (options === null || typeof options !== 'object') return undefined; - const value = (options as { serverPort?: unknown }).serverPort; - return typeof value === 'number' ? value : undefined; +function readServerPortArgv(args: unknown): number | undefined { + if (!Array.isArray(args)) return undefined; + const index = args.indexOf('-P'); + if (index === -1) return undefined; + const value = args[index + 1]; + return typeof value === 'string' ? Number(value) : undefined; } test('createAndroidPortReverseManager makes duplicate setup idempotent and cleans owner mappings', async () => { diff --git a/packages/platform-android/src/__tests__/test-utils/android-host-test-setup.ts b/packages/platform-android/src/__tests__/test-utils/android-host-test-setup.ts index 60e1f7f680..48ecc3530a 100644 --- a/packages/platform-android/src/__tests__/test-utils/android-host-test-setup.ts +++ b/packages/platform-android/src/__tests__/test-utils/android-host-test-setup.ts @@ -7,6 +7,7 @@ import { withoutCommandExecutorOverride, } from '@agent-device/host-kit/command'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; +import { lowerAndroidAdbInvocation } from '../../adb-transport.ts'; import { bindAndroidAdbHostStub } from '../../adb-host.fixtures.ts'; import { createAndroidFileHost } from './android-file-host.ts'; @@ -14,25 +15,26 @@ export function bindAndroidAdbTestHost() { return bindAndroidAdbHostStub({ environment: process.env, files: createAndroidFileHost(), - execSerialAdb: async (serial, args, options) => - await withoutCommandExecutorOverride( + execAdb: async (invocation, options) => { + const lowered = lowerAndroidAdbInvocation(invocation, options, process.env); + return await withoutCommandExecutorOverride( async () => - await runCmd('adb', ['-s', serial, ...args], { - ...options, + await runCmd('adb', lowered.args, { + ...lowered.options, detached: process.platform !== 'win32', }), - ), - spawnSerialAdb: (serial, args, options) => { - const background = runCmdBackground('adb', ['-s', serial, ...args], { - ...options, + ); + }, + spawnAdb: (invocation, options) => { + const lowered = lowerAndroidAdbInvocation(invocation, options, process.env); + const background = runCmdBackground('adb', lowered.args, { + ...lowered.options, allowFailure: true, captureOutput: false, }); void background.wait.catch(() => {}); return background.child; }, - execHostAdb: async (args, options) => - await runCmd('adb', args, { ...options, detached: process.platform !== 'win32' }), withAdbCommandExecutorOverride: withCommandExecutorOverride, withoutAdbCommandExecutorOverride: withoutCommandExecutorOverride, coerceAdbResult: coerceExecResult, diff --git a/packages/platform-android/src/adb-executor-host.test.ts b/packages/platform-android/src/adb-executor-host.test.ts index 30cc73b20d..a79fb7dfa0 100644 --- a/packages/platform-android/src/adb-executor-host.test.ts +++ b/packages/platform-android/src/adb-executor-host.test.ts @@ -1,25 +1,26 @@ import assert from 'node:assert/strict'; import { test } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; +import { parseAndroidAdbArgv } from './adb-transport.ts'; import { bindAndroidAdbHostStub } from './adb-host.fixtures.ts'; import { runAndroidHostAdb, withAndroidHostAdbTransport } from './adb-executor.ts'; test('a scoped transport intercepts host adb without reaching the injected host', async () => { let hostCalls = 0; bindAndroidAdbHostStub({ - execHostAdb: async () => { + execAdb: async () => { hostCalls += 1; return { stdout: 'host', stderr: '', exitCode: 0 }; }, }); const result = await withAndroidHostAdbTransport( - async (args, options) => { - assert.deepEqual(args, ['devices']); + async (invocation, options) => { + assert.deepEqual(invocation.command, ['devices']); assert.deepEqual(options, { timeoutMs: 1_234 }); return { stdout: 'transport', stderr: '', exitCode: 0 }; }, - async () => await runAndroidHostAdb(['devices'], { timeoutMs: 1_234 }), + async () => await runAndroidHostAdb(parseAndroidAdbArgv(['devices']), { timeoutMs: 1_234 }), ); assert.equal(result.stdout, 'transport'); @@ -29,13 +30,13 @@ test('a scoped transport intercepts host adb without reaching the injected host' test('the local host arm always obtains a result before applying the shared failure contract', async () => { let receivedOptions: Record | undefined; bindAndroidAdbHostStub({ - execHostAdb: async (_args, options) => { + execAdb: async (_invocation, options) => { receivedOptions = options; return { stdout: '', stderr: 'error: device offline', exitCode: 1 }; }, }); - const error = await runAndroidHostAdb(['devices']).then( + const error = await runAndroidHostAdb(parseAndroidAdbArgv(['devices'])).then( () => assert.fail('expected the host adb call to reject'), (error: unknown) => error, ); @@ -49,10 +50,13 @@ test('the local host arm always obtains a result before applying the shared fail test('allowFailure returns a nonzero local result unchanged', async () => { const scripted = { stdout: '', stderr: 'offline', exitCode: 7 }; - bindAndroidAdbHostStub({ execHostAdb: async () => scripted }); + bindAndroidAdbHostStub({ execAdb: async () => scripted }); assert.deepEqual( - await runAndroidHostAdb(['devices'], { allowFailure: true, timeoutMs: 5_000 }), + await runAndroidHostAdb(parseAndroidAdbArgv(['devices']), { + allowFailure: true, + timeoutMs: 5_000, + }), scripted, ); }); @@ -70,7 +74,7 @@ test('unchecked transport results are normalized at the package boundary', async const result = await withAndroidHostAdbTransport( async () => sloppy, - async () => await runAndroidHostAdb(['devices'], { allowFailure: true }), + async () => await runAndroidHostAdb(parseAndroidAdbArgv(['devices']), { allowFailure: true }), ); assert.deepEqual(result, { stdout: '', stderr: '', exitCode: 1 }); @@ -78,7 +82,7 @@ test('unchecked transport results are normalized at the package boundary', async test('nested transport scopes are innermost-first and restore on scope exit', async () => { bindAndroidAdbHostStub({ - execHostAdb: async () => ({ stdout: 'host', stderr: '', exitCode: 0 }), + execAdb: async () => ({ stdout: 'host', stderr: '', exitCode: 0 }), }); const transportFor = (name: string) => async () => ({ stdout: name, @@ -87,11 +91,11 @@ test('nested transport scopes are innermost-first and restore on scope exit', as }); await withAndroidHostAdbTransport(transportFor('outer'), async () => { - assert.equal((await runAndroidHostAdb(['devices'])).stdout, 'outer'); + assert.equal((await runAndroidHostAdb(parseAndroidAdbArgv(['devices']))).stdout, 'outer'); await withAndroidHostAdbTransport(transportFor('inner'), async () => { - assert.equal((await runAndroidHostAdb(['devices'])).stdout, 'inner'); + assert.equal((await runAndroidHostAdb(parseAndroidAdbArgv(['devices']))).stdout, 'inner'); }); - assert.equal((await runAndroidHostAdb(['devices'])).stdout, 'outer'); + assert.equal((await runAndroidHostAdb(parseAndroidAdbArgv(['devices']))).stdout, 'outer'); }); - assert.equal((await runAndroidHostAdb(['devices'])).stdout, 'host'); + assert.equal((await runAndroidHostAdb(parseAndroidAdbArgv(['devices']))).stdout, 'host'); }); diff --git a/packages/platform-android/src/adb-host.fixtures.ts b/packages/platform-android/src/adb-host.fixtures.ts index ddde542c59..4faaa7fe52 100644 --- a/packages/platform-android/src/adb-host.fixtures.ts +++ b/packages/platform-android/src/adb-host.fixtures.ts @@ -57,14 +57,11 @@ export function bindAndroidAdbHostStub( throw new Error('adb-host stub: writeBytes not stubbed'); }, }, - execSerialAdb: async () => { - throw new Error('adb-host stub: execSerialAdb not stubbed'); + execAdb: async () => { + throw new Error('adb-host stub: execAdb not stubbed'); }, - spawnSerialAdb: () => { - throw new Error('adb-host stub: spawnSerialAdb not stubbed'); - }, - execHostAdb: async () => { - throw new Error('adb-host stub: execHostAdb not stubbed'); + spawnAdb: () => { + throw new Error('adb-host stub: spawnAdb not stubbed'); }, withAdbCommandExecutorOverride: async (_override, fn) => await fn(), withoutAdbCommandExecutorOverride: async (fn) => await fn(), diff --git a/packages/platform-android/src/adb-host.ts b/packages/platform-android/src/adb-host.ts index d6873fa030..28f14be966 100644 --- a/packages/platform-android/src/adb-host.ts +++ b/packages/platform-android/src/adb-host.ts @@ -1,6 +1,7 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import type { AndroidHelperInstallDecision, AndroidImeHelperArtifact } from './helper-artifacts.ts'; import type { + AndroidAdbInvocation, AndroidAdbExecutor, AndroidAdbExecutorOptions, AndroidAdbExecutorResult, @@ -45,26 +46,18 @@ export type AndroidAdbHost = Readonly<{ /** Narrow filesystem authority used by Android helper, SDK, and artifact mechanics. */ files: AndroidAdbFileHost; /** - * Device-scoped local adb execution for `serial`, escaping any active command-executor - * override (a tunnel-backed provider shelling out to adb must not route back into itself) - * and owning the host-side process-group/teardown semantics. + * Local adb execution for one addressing decision, visible to an installed command-executor + * override so a scoped transport can answer for it. A caller that must not be captured — the + * device-scoped executor a provider would otherwise re-enter — wraps this in + * `withoutAdbCommandExecutorOverride`. The invocation's command is appended verbatim; only + * `target` is lowered into adb global options. */ - execSerialAdb( - serial: string, - args: string[], - options?: AndroidAdbExecutorOptions, - ): Promise; - /** Device-scoped local adb background spawn for `serial`; the host owns stream wiring. */ - spawnSerialAdb( - serial: string, - args: string[], - options?: AndroidAdbSpawnOptions, - ): AndroidAdbProcess; - /** Host-global adb execution (no serial), e.g. `adb devices`. */ - execHostAdb( - args: string[], + execAdb( + invocation: AndroidAdbInvocation, options?: AndroidAdbExecutorOptions, ): Promise; + /** Local adb background spawn for one addressing decision; the host owns stream wiring. */ + spawnAdb(invocation: AndroidAdbInvocation, options?: AndroidAdbSpawnOptions): AndroidAdbProcess; /** Installs `override` as the host command-executor override for the duration of `fn`. */ withAdbCommandExecutorOverride( override: AndroidAdbCommandExecutorOverride, @@ -109,9 +102,9 @@ export type AndroidAdbHost = Readonly<{ let boundHost: AndroidAdbHost | undefined; -/** Scoped override for host-global and explicitly serial-qualified adb argv. */ +/** Scoped override for host-global and explicitly serial-qualified adb invocations. */ export type AndroidAdbHostTransport = ( - args: string[], + invocation: AndroidAdbInvocation, options?: AndroidAdbExecutorOptions, ) => Promise; @@ -138,20 +131,21 @@ export function requireAndroidAdbHost(): AndroidAdbHost { * innermost-first and restore automatically. */ export async function runAndroidHostAdb( - args: string[], + invocation: AndroidAdbInvocation, options?: AndroidAdbExecutorOptions, ): Promise { const host = requireAndroidAdbHost(); const transport = androidAdbHostTransportScope.getStore(); const result = host.coerceAdbResult( transport - ? await transport(args, options) - : await host.execHostAdb(args, { ...options, allowFailure: true }), + ? await transport(invocation, options) + : await host.execAdb(invocation, { ...options, allowFailure: true }), ); if (!options?.allowFailure && result.exitCode !== 0) { const { androidAdbResultError } = await import('./adb-failure.ts'); + const { serializeAndroidAdbInvocation } = await import('./adb-transport.ts'); throw androidAdbResultError( - `adb ${args.join(' ')} exited with code ${result.exitCode}`, + `adb ${serializeAndroidAdbInvocation(invocation).join(' ')} exited with code ${result.exitCode}`, result, ); } diff --git a/packages/platform-android/src/adb-provider-scope.test.ts b/packages/platform-android/src/adb-provider-scope.test.ts index 22cacbce4b..ba81c80992 100644 --- a/packages/platform-android/src/adb-provider-scope.test.ts +++ b/packages/platform-android/src/adb-provider-scope.test.ts @@ -12,7 +12,16 @@ import { withAndroidAdbProvider, } from './adb-provider-scope.ts'; import { runAndroidHostAdb } from './adb-host.ts'; -import type { AndroidAdbExecutorResult, AndroidAdbProvider } from './adb-transport.ts'; +import { + type AndroidAdbExecutorOptions, + type AndroidAdbExecutorResult, + type AndroidAdbProvider, + androidAdbInvocation, + androidAdbSerialTarget, + parseAndroidAdbArgv, + serializeAndroidAdbInvocation, + type AndroidAdbInvocation, +} from './adb-transport.ts'; const DEVICE: DeviceInfo = { platform: 'android', @@ -25,10 +34,39 @@ const OTHER: DeviceInfo = { ...DEVICE, id: 'emulator-5556' }; const ok = (): AndroidAdbExecutorResult => ({ exitCode: 0, stdout: '', stderr: '' }); +/** The device the host port was addressed to, and the argv it will run for that device. */ +function invokedSerial(invocation: AndroidAdbInvocation): string { + if (invocation.target.selector.kind !== 'serial') { + throw new Error('expected a device-scoped adb invocation'); + } + return invocation.target.selector.serial; +} + +/** One host-route call as the scope left it: argv, the addressing's server, the option's server. */ +function hostCall(invocation: AndroidAdbInvocation, options?: AndroidAdbExecutorOptions) { + return { + args: invokedArgv(invocation), + serverPort: invokedServerPort(invocation), + optionServerPort: options?.serverPort, + }; +} + +/** A host call the lease answered for: this scope's serial, the lease's server, no option port. */ +function scoped(serial: string, ...command: string[]) { + return { args: ['-s', serial, ...command], serverPort: 15_037, optionServerPort: undefined }; +} + +function invokedArgv(invocation: AndroidAdbInvocation): string[] { + return serializeAndroidAdbInvocation({ + ...invocation, + target: { ...invocation.target, server: { kind: 'ambient' } }, + }); +} + test('resolution answers from the installed scope for the matching serial only', async () => { bindAndroidAdbHostStub({ - execSerialAdb: async (serial) => { - throw new Error(`local adb must not run in this test (serial ${serial})`); + execAdb: async (invocation) => { + throw new Error(`local adb must not run in this test (serial ${invokedSerial(invocation)})`); }, }); const provider: AndroidAdbProvider = { @@ -50,10 +88,10 @@ test('resolution answers from the installed scope for the matching serial only', }); test('outside any scope, resolution falls back to host adb for the device serial', async () => { - const serialCalls: Array<[string, string[]]> = []; + const serialCalls: Array<[string, readonly string[]]> = []; bindAndroidAdbHostStub({ - execSerialAdb: async (serial, args) => { - serialCalls.push([serial, args]); + execAdb: async (invocation) => { + serialCalls.push([invokedSerial(invocation), invocation.command]); return ok(); }, }); @@ -97,11 +135,129 @@ test('the installed override routes only normalized device-scoped adb calls to t expect(providerCalls).toEqual([['shell', 'ls']]); }); +test('a managed port scope refuses global options the provider cannot restate', async () => { + const providerCalls: string[][] = []; + const provider: AndroidAdbProvider = { + exec: async (args) => { + providerCalls.push(args); + return ok(); + }, + }; + const capture = async (scope: { serial: string; serverPort?: number }, args: string[]) => { + let captured: + | ((cmd: string, args: string[], options: object) => Promise | undefined) + | undefined; + bindAndroidAdbHostStub({ + withAdbCommandExecutorOverride: async (override, fn) => { + captured = override; + return await fn(); + }, + }); + await withAndroidAdbProvider(provider, scope, async () => { + captured?.('adb', args, {}); + }); + }; + + // A private adb server cannot carry a caller's `-t`, so the call is refused and the provider + // never answers for an addressing set it did not choose. + await expect( + capture({ serial: DEVICE.id, serverPort: 15_037 }, [ + '-t', + '42', + '-s', + DEVICE.id, + 'shell', + 'ls', + ]), + ).rejects.toMatchObject({ details: { reason: 'managed-device-transport-mismatch' } }); + expect(providerCalls).toEqual([]); + + // A port typed into argv names a server the provider cannot address, so the server rule refuses + // it here — not the host-global rule, which is why the port has to parse. + providerCalls.length = 0; + await expect( + capture({ serial: DEVICE.id, serverPort: 15_037 }, [ + '-P', + '9999', + '-s', + DEVICE.id, + 'shell', + 'ls', + ]), + ).rejects.toThrowError(/cannot select another server/); + expect(providerCalls).toEqual([]); + + // The server this lease holds is the one port the provider may be handed a request for, argv + // included, and the request still travels as the caller wrote it. + await capture({ serial: DEVICE.id, serverPort: 15_037 }, [ + '-P', + '15037', + '-s', + DEVICE.id, + 'shell', + 'ls', + ]); + expect(providerCalls).toEqual([['-P', '15037', 'shell', 'ls']]); + + // Without a lease the caller's own adb invocation is what runs, globals and all: the provider + // receives the request with only this scope's `-s` pair removed, and no server rule applies. + providerCalls.length = 0; + await capture({ serial: DEVICE.id }, ['-t', '42', '-s', DEVICE.id, 'shell', 'ls']); + await capture({ serial: DEVICE.id }, ['-P', '9999', '-s', DEVICE.id, 'shell', 'ls']); + expect(providerCalls).toEqual([ + ['-t', '42', 'shell', 'ls'], + ['-P', '9999', 'shell', 'ls'], + ]); +}); + +test('the provider receives the caller request with only the scope serial removed', async () => { + const providerCalls: string[][] = []; + const hostCalls: ReturnType[] = []; + const provider: AndroidAdbProvider = { + exec: async (args) => { + providerCalls.push(args); + return ok(); + }, + }; + let captured: + | ((cmd: string, args: string[], options: object) => Promise | undefined) + | undefined; + bindAndroidAdbHostStub({ + execAdb: async (invocation, options) => { + hostCalls.push(hostCall(invocation, options)); + return ok(); + }, + withAdbCommandExecutorOverride: async (override, fn) => { + captured = override; + return await fn(); + }, + }); + + await withAndroidAdbProvider(provider, { serial: DEVICE.id, serverPort: 15_037 }, async () => { + // A readiness token is part of the request the provider must honor, not addressing it owns, + // so it travels ahead of the command instead of being parsed away. + captured?.('adb', ['-s', DEVICE.id, 'wait-for-device', 'shell', 'getprop'], {}); + // A call that addresses no device is not the provider's to answer as a device command. + captured?.('adb', ['get-state'], {}); + }); + // A transport global under a lease is refused by the test above, not restated here. + await withAndroidAdbProvider(provider, { serial: DEVICE.id }, async () => { + captured?.('adb', ['-d', '-s', DEVICE.id, 'shell', 'getprop'], {}); + }); + + expect(providerCalls).toEqual([ + ['wait-for-device', 'shell', 'getprop'], + ['-d', 'shell', 'getprop'], + ]); + // A server-level command addresses no device, so the lease answers it on its own transport. + expect(hostCalls).toEqual([scoped(DEVICE.id, 'get-state')]); +}); + test('a managed port scope rejects foreign serials before host adb execution', async () => { - const hostCalls: Array<{ args: string[]; serverPort?: number }> = []; + const hostCalls: ReturnType[] = []; bindAndroidAdbHostStub({ - execHostAdb: async (args, options) => { - hostCalls.push({ args, serverPort: options?.serverPort }); + execAdb: async (invocation, options) => { + hostCalls.push(hostCall(invocation, options)); return ok(); }, }); @@ -110,21 +266,30 @@ test('a managed port scope rejects foreign serials before host adb execution', a { exec: async () => ok() }, { serial: DEVICE.id, serverPort: 15_037 }, async () => { - await runAndroidHostAdb(['devices']); - await runAndroidHostAdb(['shell', 'id'], { env: { ANDROID_SERIAL: OTHER.id } }); - await runAndroidHostAdb(['-s', DEVICE.id, 'shell', 'getprop']); - await expect(runAndroidHostAdb(['-s', OTHER.id, 'shell', 'getprop'])).rejects.toMatchObject({ + await runAndroidHostAdb(parseAndroidAdbArgv(['devices'])); + await runAndroidHostAdb(parseAndroidAdbArgv(['shell', 'id']), { + env: { ANDROID_SERIAL: OTHER.id }, + }); + await runAndroidHostAdb( + androidAdbInvocation(androidAdbSerialTarget(DEVICE.id), ['shell', 'getprop']), + ); + await expect( + runAndroidHostAdb( + androidAdbInvocation(androidAdbSerialTarget(OTHER.id), ['shell', 'getprop']), + ), + ).rejects.toMatchObject({ details: { reason: 'managed-device-transport-mismatch' }, }); }, ); - await runAndroidHostAdb(['devices']); + await runAndroidHostAdb(parseAndroidAdbArgv(['devices'])); + // The lease's server rides in the addressing, so no per-call option can move it afterwards. expect(hostCalls).toEqual([ - { args: ['-s', DEVICE.id, 'devices'], serverPort: 15_037 }, - { args: ['-s', DEVICE.id, 'shell', 'id'], serverPort: 15_037 }, - { args: ['-s', DEVICE.id, 'shell', 'getprop'], serverPort: 15_037 }, - { args: ['devices'] }, + scoped(DEVICE.id, 'devices'), + scoped(DEVICE.id, 'shell', 'id'), + scoped(DEVICE.id, 'shell', 'getprop'), + { args: ['devices'], serverPort: undefined, optionServerPort: undefined }, ]); }); @@ -135,8 +300,8 @@ test('a managed port scope classifies absolute adb commands and preserves the de | ((cmd: string, args: string[], options: object) => Promise | undefined) | undefined; bindAndroidAdbHostStub({ - execHostAdb: async (args) => { - hostCalls.push(args); + execAdb: async (invocation) => { + hostCalls.push(invokedArgv(invocation)); return ok(); }, withAdbCommandExecutorOverride: async (override, fn) => { @@ -199,12 +364,18 @@ test('managed port scopes refuse foreign device resolvers before returning a loc test('private-port execution contains local transports constructed before entering the scope', async () => { const calls: Array<{ serial: string; serverPort?: number }> = []; bindAndroidAdbHostStub({ - execSerialAdb: async (serial, _args, options) => { - calls.push({ serial, serverPort: options?.serverPort }); + execAdb: async (invocation, options) => { + calls.push({ + serial: invokedSerial(invocation), + serverPort: options?.serverPort ?? invokedServerPort(invocation), + }); return ok(); }, - spawnSerialAdb: (serial, _args, options) => { - calls.push({ serial, serverPort: options?.serverPort }); + spawnAdb: (invocation, options) => { + calls.push({ + serial: invokedSerial(invocation), + serverPort: options?.serverPort ?? invokedServerPort(invocation), + }); return undefined as never; }, }); @@ -234,14 +405,90 @@ test('private-port execution contains local transports constructed before enteri ]); }); +test('a leased host transport answers for its own server, and refuses one a call names', async () => { + const hostCalls: ReturnType[] = []; + bindAndroidAdbHostStub({ + execAdb: async (invocation, options) => { + hostCalls.push(hostCall(invocation, options)); + return ok(); + }, + }); + + await withAndroidAdbProvider( + { exec: async () => ok() }, + { serial: DEVICE.id, serverPort: 15_037 }, + async () => { + await runAndroidHostAdb(parseAndroidAdbArgv(['devices']), { serverPort: 15_037 }); + await expect( + runAndroidHostAdb(parseAndroidAdbArgv(['devices']), { serverPort: 9_999 }), + ).rejects.toMatchObject({ + details: { reason: 'managed-device-transport-mismatch' }, + }); + }, + ); + + expect(hostCalls).toEqual([scoped(DEVICE.id, 'devices')]); +}); + +test('a device route answers for the server it was built with, not one a call names', async () => { + const calls: ReturnType[] = []; + bindAndroidAdbHostStub({ + execAdb: async (invocation, options) => { + calls.push(hostCall(invocation, options)); + return ok(); + }, + spawnAdb: (invocation, options) => { + calls.push(hostCall(invocation, options)); + return undefined as never; + }, + }); + + // With no lease the port the route was built with is the one it addresses, so a per-call option + // is not a second channel that can move the same request onto another adb server. + const route = createLocalAndroidAdbProvider(DEVICE, { serverPort: 15_037 }); + await route.exec(['shell', 'id'], { serverPort: 9_999 }); + route.spawn?.(['logcat'], { serverPort: 9_999 }); + expect(calls).toEqual([scoped(DEVICE.id, 'shell', 'id'), scoped(DEVICE.id, 'logcat')]); + + // Under a lease the same request is refused rather than answered on a server the lease lost. + calls.length = 0; + await withAndroidAdbProvider( + { exec: async () => ok() }, + { serial: DEVICE.id, serverPort: 15_037 }, + async () => { + await expect(route.exec(['shell', 'id'], { serverPort: 9_999 })).rejects.toMatchObject({ + details: { reason: 'managed-device-transport-mismatch' }, + }); + expect(() => route.spawn?.(['logcat'], { serverPort: 9_999 })).toThrowError( + expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }), + ); + }, + ); + expect(calls).toEqual([]); + + // A port typed into argv names the same conflict as one passed as an option, and the route has to + // answer for it too: it is the arm that would otherwise overwrite the caller's `-P`. + await expect(route.exec(['-P', '9999', 'shell', 'id'])).rejects.toMatchObject({ + details: { reason: 'managed-device-transport-mismatch' }, + }); + expect(() => route.spawn?.(['-P', '9999', 'logcat'])).toThrowError( + expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }), + ); + expect(calls).toEqual([]); + + // The port this route was built with is the one it may answer for, however the caller spells it. + await route.exec(['-P', '15037', 'shell', 'id']); + expect(calls).toEqual([scoped(DEVICE.id, 'shell', 'id')]); +}); + test('a managed port scope keeps shell -s arguments on the private transport', async () => { - const hostCalls: Array<{ args: string[]; serverPort?: number }> = []; + const hostCalls: ReturnType[] = []; let captured: | ((cmd: string, args: string[], options: object) => Promise | undefined) | undefined; bindAndroidAdbHostStub({ - execHostAdb: async (args, options) => { - hostCalls.push({ args, serverPort: options?.serverPort }); + execAdb: async (invocation, options) => { + hostCalls.push(hostCall(invocation, options)); return ok(); }, withAdbCommandExecutorOverride: async (override, fn) => { @@ -254,7 +501,7 @@ test('a managed port scope keeps shell -s arguments on the private transport', a { exec: async () => ok() }, { serial: DEVICE.id, serverPort: 15_037 }, async () => { - await runAndroidHostAdb(['shell', 'echo', '-s', OTHER.id]); + await runAndroidHostAdb(parseAndroidAdbArgv(['shell', 'echo', '-s', OTHER.id])); const shellCommand = captured?.('adb', ['shell', 'echo', '-s', OTHER.id], {}); expect(shellCommand).toBeDefined(); await shellCommand; @@ -262,16 +509,16 @@ test('a managed port scope keeps shell -s arguments on the private transport', a ); expect(hostCalls).toEqual([ - { args: ['-s', DEVICE.id, 'shell', 'echo', '-s', OTHER.id], serverPort: 15_037 }, - { args: ['-s', DEVICE.id, 'shell', 'echo', '-s', OTHER.id], serverPort: 15_037 }, + scoped(DEVICE.id, 'shell', 'echo', '-s', OTHER.id), + scoped(DEVICE.id, 'shell', 'echo', '-s', OTHER.id), ]); }); test('a managed port scope restores the default transport after task failure', async () => { const ports: Array = []; bindAndroidAdbHostStub({ - execHostAdb: async (_args, options) => { - ports.push(options?.serverPort); + execAdb: async (invocation, options) => { + ports.push(options?.serverPort ?? invokedServerPort(invocation)); return ok(); }, }); @@ -281,21 +528,29 @@ test('a managed port scope restores the default transport after task failure', a { exec: async () => ok() }, { serial: DEVICE.id, serverPort: 15_037 }, async () => { - await runAndroidHostAdb(['devices']); + await runAndroidHostAdb(parseAndroidAdbArgv(['devices'])); throw new Error('stop managed request'); }, ), ).rejects.toThrow('stop managed request'); - await runAndroidHostAdb(['devices']); + await runAndroidHostAdb(parseAndroidAdbArgv(['devices'])); expect(ports).toEqual([15_037, undefined]); }); test('a managed port scope carries its server through the local background transport', async () => { - const spawnCalls: Array<{ serial: string; args: string[]; serverPort?: number }> = []; + const spawnCalls: Array<{ + serial: string; + args: readonly string[]; + serverPort?: number; + }> = []; bindAndroidAdbHostStub({ - spawnSerialAdb: (serial, args, options) => { - spawnCalls.push({ serial, args, serverPort: options?.serverPort }); + spawnAdb: (invocation, options) => { + spawnCalls.push({ + serial: invokedSerial(invocation), + args: invocation.command, + serverPort: options?.serverPort ?? invokedServerPort(invocation), + }); return undefined as never; }, }); @@ -321,10 +576,10 @@ test('a managed port scope carries its server through the local background trans test('managed port scopes remain isolated across concurrent requests', async () => { const hostCalls: Array<{ serial: string; serverPort?: number }> = []; bindAndroidAdbHostStub({ - execHostAdb: async (args, options) => { + execAdb: async (invocation, options) => { hostCalls.push({ - serial: args[args.indexOf('-s') + 1] ?? 'global', - serverPort: options?.serverPort, + serial: invokedSerial(invocation), + serverPort: options?.serverPort ?? invokedServerPort(invocation), }); await Promise.resolve(); return ok(); @@ -335,12 +590,18 @@ test('managed port scopes remain isolated across concurrent requests', async () withAndroidAdbProvider( { exec: async () => ok() }, { serial: DEVICE.id, serverPort: 15_037 }, - async () => await runAndroidHostAdb(['-s', DEVICE.id, 'shell', 'id']), + async () => + await runAndroidHostAdb( + androidAdbInvocation(androidAdbSerialTarget(DEVICE.id), ['shell', 'id']), + ), ), withAndroidAdbProvider( { exec: async () => ok() }, { serial: OTHER.id, serverPort: 15_038 }, - async () => await runAndroidHostAdb(['-s', OTHER.id, 'shell', 'id']), + async () => + await runAndroidHostAdb( + androidAdbInvocation(androidAdbSerialTarget(OTHER.id), ['shell', 'id']), + ), ), ]); @@ -349,3 +610,7 @@ test('managed port scopes remain isolated across concurrent requests', async () { serial: OTHER.id, serverPort: 15_038 }, ]); }); + +function invokedServerPort(invocation: AndroidAdbInvocation): number | undefined { + return invocation.target.server.kind === 'port' ? invocation.target.server.port : undefined; +} diff --git a/packages/platform-android/src/adb-provider-scope.ts b/packages/platform-android/src/adb-provider-scope.ts index d2a63b2b95..4c8ac4b28f 100644 --- a/packages/platform-android/src/adb-provider-scope.ts +++ b/packages/platform-android/src/adb-provider-scope.ts @@ -1,27 +1,38 @@ import { AsyncLocalStorage } from 'node:async_hooks'; import path from 'node:path'; import type { DeviceInfo } from '@agent-device/kernel/device'; -import { AppError } from '@agent-device/kernel/errors'; -import { - requireAndroidAdbHost, - withAndroidHostAdbTransport, - type AndroidAdbCommandExecutorOverride, - type AndroidAdbHostTransport, -} from './adb-host.ts'; -import { withAdbFailureHints } from './adb-failure.ts'; -import { createExecAndroidPortReverseProvider } from './adb-port-reverse.ts'; -import { normalizeAndroidAdbProvider } from './adb-provider-normalization.ts'; import { + androidAdbInvocation, + androidAdbPayloadWithoutSerial, + applyManagedAndroidAdbServer, normalizeAndroidAdbInstallOptions, + parseAndroidAdbArgv, + adoptAndroidAdbSerial, + requireManagedAndroidAdbAddressing, + requireManagedAndroidAdbSerial, + requireSameAndroidAdbServer, + requireUnconflictedAndroidAdbSelector, type AndroidAdbExecutor, type AndroidAdbExecutorOptions, + type AndroidAdbExecutorResult, + type AndroidAdbInvocation, type AndroidAdbProvider, type AndroidAdbProviderScopeOptions, + type AndroidAdbSelector, type AndroidAdbSpawner, type AndroidTextInjector, type AndroidTouchProvider, type ScopedAndroidAdbBackgroundTransport, } from './adb-transport.ts'; +import { + requireAndroidAdbHost, + withAndroidHostAdbTransport, + type AndroidAdbCommandExecutorOverride, + type AndroidAdbHostTransport, +} from './adb-host.ts'; +import { withAdbFailureHints } from './adb-failure.ts'; +import { createExecAndroidPortReverseProvider } from './adb-port-reverse.ts'; +import { normalizeAndroidAdbProvider } from './adb-provider-normalization.ts'; // The request-scoped provider seam: withAndroidAdbProvider installs a provider for one device // serial, and every resolver below answers from that scope — falling back to host adb through @@ -44,26 +55,89 @@ export function createDeviceAdbExecutor( function createSerialAdbExecutor(serial: string, serverPort?: number): AndroidAdbExecutor { return withAdbFailureHints(async (args, options) => { - const port = scopedServerPort(serial, serverPort); - return await requireAndroidAdbHost().execSerialAdb( - serial, - args, - port === undefined ? options : { ...options, serverPort: port }, + const request = deviceAdbRouteRequest(serial, serverPort, args, options); + // A device-scoped executor is the terminal local route: an installed provider must not + // capture it and route the call back into itself. + return await requireAndroidAdbHost().withoutAdbCommandExecutorOverride( + async () => await requireAndroidAdbHost().execAdb(request.invocation, request.options), ); }); } function createSerialAdbSpawner(serial: string, serverPort?: number): AndroidAdbSpawner { return (args, options) => { - const port = scopedServerPort(serial, serverPort); - return requireAndroidAdbHost().spawnSerialAdb( + const request = deviceAdbRouteRequest(serial, serverPort, args, options); + return requireAndroidAdbHost().spawnAdb(request.invocation, request.options); + }; +} + +/** One device-scoped request: addressing decided once, and the server's option channel removed. */ +function deviceAdbRouteRequest( + serial: string, + installed: number | undefined, + args: string[], + options: Options | undefined, +): { invocation: AndroidAdbInvocation; options: Omit } { + const { serverPort: requested, ...rest } = options ?? ({} as Options); + return { + invocation: androidDeviceAdbInvocation( serial, args, - port === undefined ? options : { ...options, serverPort: port }, - ); + deviceServerPort(serial, installed, requested), + ), + options: rest, }; } +/** + * Addresses `args` for `serial`, carrying the chosen adb server on the target and nowhere else. A + * managed transport re-reads the argv through the shared managed rules, so the payload it spawns + * is the parsed command by reference. A caller-chosen server rewrites addressing too, and an + * ambient transport keeps the caller's argv as the emitted form, because ambient adb lets a later + * `-s` win. + */ +function androidDeviceAdbInvocation( + serial: string, + args: string[], + port: number | undefined, +): AndroidAdbInvocation { + const parsed = parseAndroidAdbArgv(args); + const selector = adoptAndroidAdbSerial(parsed.target, serial); + if (port === undefined) { + return androidAdbInvocation(selector, parsed.command, ['-s', serial, ...args]); + } + // A private adb server makes this a managed transport, whoever named the port: the rules over + // what such a transport may be asked for are the same ones the lease is held to. + const managed = applyManagedAndroidAdbServer(parsed, { port }); + return androidAdbInvocation( + requireManagedAndroidAdbSerial(managed.target, serial), + managed.command, + ); +} + +/** + * The adb server a device-scoped route runs against, from the one channel that may name it. + * + * Under a lease that is the lease's private server and nothing else: a route constructed for + * another port, or a call asking for one, is refused rather than followed. Without a lease the + * owner's port wins over a per-call request, which is what the route was built to answer for. + */ +function deviceServerPort( + serial: string, + installed: number | undefined, + requested: number | undefined, +): number | undefined { + const scope = androidAdbProviderScope.getStore(); + if (scope) requireScopedSerial(scope, { kind: 'serial', serial }); + if (scope?.serverPort !== undefined) { + // A lease's server is the only one this route may answer for; naming another, at construction + // or per call, is refused rather than followed. + requireSameAndroidAdbServer(installed, scope.serverPort); + return requireSameAndroidAdbServer(requested, scope.serverPort); + } + return installed ?? requested; +} + export function createLocalAndroidAdbProvider( device: DeviceInfo, options: Readonly<{ serverPort?: number }> = {}, @@ -163,7 +237,10 @@ export async function withAndroidAdbProvider( async () => await requireAndroidAdbHost().withAdbCommandExecutorOverride(override, fn), ); if (options.serverPort === undefined) return await run(); - return await withAndroidHostAdbTransport(createScopedHostTransport(scope), run); + return await withAndroidHostAdbTransport( + createScopedHostTransport(scope, options.serverPort), + run, + ); } function createAndroidCommandExecutorOverride( @@ -172,107 +249,82 @@ function createAndroidCommandExecutorOverride( return (cmd, args, options) => { if (!isAdbCommand(cmd)) return undefined; if (scope.serverPort === undefined && cmd !== 'adb') return undefined; - const serial = readAdbSerial(args); - requireScopedSerial(scope, serial); - if (serial && serial !== scope.serial) return undefined; - if (serial === scope.serial) { - const providerArgs = stripAdbSerialArgs(args, scope.serial); - if (!providerArgs) return undefined; + const invocation = parseAndroidAdbArgv(args); + requireScopedSerial(scope, invocation.target.selector); + if (invocation.target.selector.kind === 'serial') { + if (invocation.target.selector.serial !== scope.serial) return undefined; + // Under a private adb server the provider cannot restate a caller's host globals, and a + // `-P` naming another server would reach adb through a provider that never sees addressing, + // so both are refused here. Without a lease the caller's own adb invocation is what runs, as + // it always has. + if (scope.serverPort !== undefined) { + requireManagedAndroidAdbAddressing(invocation.target, scope.serverPort); + } + // The provider contract is argv-shaped, so it receives the caller's request with this + // scope's own `-s` pair removed — readiness tokens and transport globals left where the + // caller put them, and never a rebuild with the scope's serial stitched back in. + const payload = androidAdbPayloadWithoutSerial(args, scope.serial); + if (payload === undefined) return undefined; return requireAndroidAdbHost().withoutAdbCommandExecutorOverride( - async () => await scope.provider.exec(providerArgs, options), + async () => await scope.provider.exec(payload, options), ); } - if (scope.serverPort === undefined) return undefined; - return requireAndroidAdbHost().withoutAdbCommandExecutorOverride( - async () => - await requireAndroidAdbHost().execHostAdb(['-s', scope.serial, ...args], { - ...options, - allowFailure: true, - serverPort: scope.serverPort, - }), - ); + const port = scope.serverPort; + if (port === undefined) return undefined; + return execOnScopedTransport(scope, port, invocation, options); }; } -function createScopedHostTransport(scope: AndroidAdbProviderScope): AndroidAdbHostTransport { - return async (args: string[], options?: AndroidAdbExecutorOptions) => { - const serial = readAdbSerial(args); - requireScopedSerial(scope, serial); - const host = requireAndroidAdbHost(); - return await host.withoutAdbCommandExecutorOverride( - async () => - await host.execHostAdb(serial === undefined ? ['-s', scope.serial, ...args] : args, { - ...options, - allowFailure: true, - serverPort: scope.serverPort, - }), - ); +function createScopedHostTransport( + scope: AndroidAdbProviderScope, + port: number, +): AndroidAdbHostTransport { + return async (invocation, options) => { + requireScopedSerial(scope, invocation.target.selector); + return execOnScopedTransport(scope, port, invocation, options); }; } +/** + * Answers one request on the lease's own transport: the scope's serial, and its server carried in + * the addressing rather than in the options, which is what leaves a caller no second way to name + * another adb server. A call that names one anyway is refused, not quietly restated. + */ +async function execOnScopedTransport( + scope: AndroidAdbProviderScope, + port: number, + invocation: AndroidAdbInvocation, + options: AndroidAdbExecutorOptions | undefined, +): Promise { + requireSameAndroidAdbServer(port, options?.serverPort); + const { serverPort: _omitted, ...rest } = options ?? {}; + const serialTarget = requireManagedAndroidAdbSerial(invocation.target, scope.serial); + const scoped = applyManagedAndroidAdbServer( + androidAdbInvocation(serialTarget, invocation.command), + { port }, + ); + const host = requireAndroidAdbHost(); + return await host.withoutAdbCommandExecutorOverride( + async () => await host.execAdb(scoped, { ...rest, allowFailure: true }), + ); +} + function scopeForDevice(device: DeviceInfo): AndroidAdbProviderScope | undefined { const scoped = androidAdbProviderScope.getStore(); - requireScopedSerial(scoped, device.id); + if (scoped) requireScopedSerial(scoped, { kind: 'serial', serial: device.id }); return scoped; } +/** Under a private server, a request that names another device is not this scope's to answer. */ function requireScopedSerial( scope: AndroidAdbProviderScope | undefined, - serial: string | undefined, -) { - if (scope?.serverPort !== undefined && serial !== undefined && serial !== scope.serial) { - throw new AppError('COMMAND_FAILED', 'Managed ADB transport cannot address another device.', { - reason: 'managed-device-transport-mismatch', - }); - } -} - -function scopedServerPort(serial: string, requested: number | undefined): number | undefined { - const scope = androidAdbProviderScope.getStore(); - requireScopedSerial(scope, serial); - if (scope?.serverPort === undefined) return requested; - if (requested !== undefined && requested !== scope.serverPort) { - throw new AppError('COMMAND_FAILED', 'Managed ADB transport cannot select another server.', { - reason: 'managed-device-transport-mismatch', - }); - } - return scope.serverPort; + selector: AndroidAdbSelector, +): void { + if (scope?.serverPort === undefined) return; + requireUnconflictedAndroidAdbSelector(selector, scope.serial); } function isAdbCommand(command: string): boolean { const executable = path.basename(command).replace(/\.(?:com|exe|bat|cmd)$/i, ''); return executable === 'adb'; } - -function readAdbSerial(args: readonly string[]): string | undefined { - const serialIndex = findAdbSerialIndex(args); - return serialIndex === undefined ? undefined : args[serialIndex + 1]; -} - -function findAdbSerialIndex(args: readonly string[]): number | undefined { - let index = 0; - while (index < args.length) { - const argument = args[index]; - if (argument === '-s') return index; - if (argument === '-P' || argument === '-H' || argument === '-L') { - index += 2; - continue; - } - if (argument === '-a' || argument === '-d' || argument === '-e') { - index += 1; - continue; - } - return undefined; - } - return undefined; -} - -function stripAdbSerialArgs(args: string[], expectedSerial: string): string[] | undefined { - // The provider scope only owns normalized device-scoped adb calls: - // adb -s . Global commands - // such as adb devices/version, calls for another serial, and host-preconfigured - // invocations stay local. - const serialIndex = findAdbSerialIndex(args); - if (serialIndex === undefined || args[serialIndex + 1] !== expectedSerial) return undefined; - return [...args.slice(0, serialIndex), ...args.slice(serialIndex + 2)]; -} diff --git a/packages/platform-android/src/adb-transport.test.ts b/packages/platform-android/src/adb-transport.test.ts new file mode 100644 index 0000000000..58fa3c34ab --- /dev/null +++ b/packages/platform-android/src/adb-transport.test.ts @@ -0,0 +1,417 @@ +import { describe, expect, it } from 'vitest'; +import { + ADB_GLOBAL_OPTIONS, + ADB_MANAGED_FORBIDDEN_COMMANDS, + ADB_WAIT_STATES, + ADB_WAIT_TRANSPORTS, + androidAdbHostTarget, + androidAdbInvocation, + androidAdbPayloadWithoutSerial, + androidAdbSerialTarget, + type AndroidAdbExecutorOptions, + androidManagedAdbEnvironment, + lowerAndroidAdbInvocation, + requireSameAndroidAdbServer, + applyManagedAndroidAdbServer, + parseAndroidAdbArgv, + adoptAndroidAdbSerial, + requireAndroidAdbServerPort, + requireManagedAndroidAdbCommand, + requireManagedAndroidAdbSerial, + serializeAndroidAdbInvocation, +} from './adb-transport.ts'; + +describe('adb argv grammar', () => { + it('carries the arity adb documents for every global option', () => { + expect(ADB_GLOBAL_OPTIONS).toEqual({ + '-a': 0, + '-d': 0, + '-e': 0, + '-s': 1, + '-t': 1, + '-H': 1, + '-P': 1, + '-L': 1, + '--one-device': 1, + '--exit-on-write-error': 0, + }); + }); + + it('reads every transport-and-state product adb documents as addressing', () => { + const forms = ADB_WAIT_TRANSPORTS.flatMap((transport) => + ADB_WAIT_STATES.map((state) => `wait-for-${transport}${state}`), + ); + expect(forms).toHaveLength(20); + for (const token of forms) { + expect(parseAndroidAdbArgv([token, 'get-state']).target.waitFor).toBe(token); + } + }); + + it('hands a readiness token adb cannot parse to the command guard, not to addressing', () => { + // adb still waits for this token, so the grammar must not claim it, and must not lose it. + const invocation = parseAndroidAdbArgv(['wait-for-magic', 'get-state']); + expect(invocation.target.waitFor).toBeUndefined(); + expect(invocation.target.hostGlobals).toEqual(['wait-for-magic']); + expect(invocation.command).toEqual(['get-state']); + }); + + it('names server and transport lifecycle as forbidden under a managed transport', () => { + expect([...ADB_MANAGED_FORBIDDEN_COMMANDS].sort()).toEqual( + [ + 'attach', + 'connect', + 'detach', + 'disconnect', + 'fork-server', + 'kill-server', + 'nodaemon', + 'pair', + 'reconnect', + 'server', + 'start-server', + ].sort(), + ); + }); +}); + +describe('parseAndroidAdbArgv', () => { + it('addresses no device when the argv named none', () => { + const invocation = parseAndroidAdbArgv(['devices']); + expect(invocation.target.selector).toEqual({ kind: 'unspecified' }); + expect(invocation.command).toEqual(['devices']); + }); + + it('reads the serial an adb invocation addresses itself by', () => { + const invocation = parseAndroidAdbArgv(['-s', 'emulator-5554', 'shell', 'id']); + expect(invocation.target).toEqual({ + selector: { kind: 'serial', serial: 'emulator-5554' }, + server: { kind: 'ambient' }, + }); + expect(invocation.command).toEqual(['shell', 'id']); + }); + + it('hands an addressing-free command through by reference', () => { + const argv = ['shell', 'input', 'tap', '10', '20']; + const invocation = parseAndroidAdbArgv(argv); + expect(invocation.command).toBe(argv); + expect(invocation.rawArgv).toBeUndefined(); + }); + + it('keeps global options it does not own out of the command', () => { + const invocation = parseAndroidAdbArgv(['-t', '42', '-s', 'A', 'shell', 'id']); + expect(invocation.target.selector).toEqual({ kind: 'serial', serial: 'A' }); + expect(invocation.target.hostGlobals).toEqual(['-t', '42']); + expect(invocation.command).toEqual(['shell', 'id']); + }); + + it('stops at an option whose shape it cannot trust', () => { + const invocation = parseAndroidAdbArgv(['-s', 'A', 'shell', '-s', 'B']); + expect(invocation.command).toEqual(['shell', '-s', 'B']); + }); + + it('lets a second serial keep its winning position on the way out', () => { + const invocation = parseAndroidAdbArgv(['-s', 'A', '-s', 'B', 'shell', 'id']); + expect(invocation.target.selector).toEqual({ kind: 'serial', serial: 'A' }); + expect(invocation.target.hostGlobals).toEqual(['-s', 'B']); + }); + + it('reads a server port and a readiness request as addressing', () => { + const invocation = parseAndroidAdbArgv(['-P', '5038', 'wait-for-usb-device', 'get-state']); + expect(invocation.target.server).toEqual({ kind: 'port', port: 5038 }); + expect(invocation.target.waitFor).toBe('wait-for-usb-device'); + expect(invocation.command).toEqual(['get-state']); + }); + + it('leaves a malformed port to the process', () => { + const invocation = parseAndroidAdbArgv(['-P', 'nope', 'shell', 'id']); + expect(invocation.target.server).toEqual({ kind: 'ambient' }); + expect(invocation.target.hostGlobals).toEqual(['-P', 'nope']); + }); + + it('remembers the argv an ambient request was spelled with', () => { + const argv = ['-H', '10.0.0.8', '-P', '5037', '-s', 'A', 'shell', 'id']; + const invocation = parseAndroidAdbArgv(argv); + expect(invocation.rawArgv).toBe(argv); + expect(serializeAndroidAdbInvocation(invocation)).toEqual(argv); + }); + + it('never rewrites a payload it only read', () => { + const argv = ['-s', 'A', 'shell', 'wm', 'size']; + expect(serializeAndroidAdbInvocation(parseAndroidAdbArgv(argv))).toEqual(argv); + expect(argv).toEqual(['-s', 'A', 'shell', 'wm', 'size']); + }); + + it('drops a repeated serial that asks for nothing the first one did not say', () => { + const invocation = parseAndroidAdbArgv(['-s', 'A', '-s', 'A', 'shell', 'id']); + expect(invocation.target.selector).toEqual({ kind: 'serial', serial: 'A' }); + expect(invocation.target.hostGlobals).toBeUndefined(); + }); + + it('keeps a second readiness token as a global one typed wait cannot answer for', () => { + const invocation = parseAndroidAdbArgv(['wait-for-device', 'wait-for-usb-device', 'get-state']); + expect(invocation.target.waitFor).toBe('wait-for-device'); + expect(invocation.target.hostGlobals).toEqual(['wait-for-usb-device']); + expect(invocation.command).toEqual(['get-state']); + }); +}); + +describe('androidAdbPayloadWithoutSerial', () => { + it('carries a readiness token to whoever is asked to run the command', () => { + expect( + androidAdbPayloadWithoutSerial(['-s', 'A', 'wait-for-device', 'shell', 'getprop'], 'A'), + ).toEqual(['wait-for-device', 'shell', 'getprop']); + }); + + it('leaves globals the caller spelled where the caller spelled them', () => { + expect(androidAdbPayloadWithoutSerial(['-t', '42', '-s', 'A', 'shell', 'id'], 'A')).toEqual([ + '-t', + '42', + 'shell', + 'id', + ]); + }); + + it('answers undefined for a request addressing another device, none, or only a payload -s', () => { + expect(androidAdbPayloadWithoutSerial(['-s', 'B', 'shell', 'id'], 'A')).toBeUndefined(); + expect(androidAdbPayloadWithoutSerial(['get-state'], 'A')).toBeUndefined(); + expect(androidAdbPayloadWithoutSerial(['shell', 'echo', '-s', 'A'], 'A')).toBeUndefined(); + }); +}); + +describe('requireSameAndroidAdbServer', () => { + it('keeps the server one layer already owns', () => { + expect(requireSameAndroidAdbServer(15_037, undefined)).toBe(15_037); + expect(requireSameAndroidAdbServer(undefined, 15_037)).toBe(15_037); + expect(requireSameAndroidAdbServer(15_037, 15_037)).toBe(15_037); + expect(requireSameAndroidAdbServer(undefined, undefined)).toBeUndefined(); + }); + + it('refuses a second name for the server, whoever asked', () => { + expect(() => requireSameAndroidAdbServer(15_037, 9_999)).toThrowError( + expect.objectContaining({ + code: 'COMMAND_FAILED', + message: 'Managed ADB transport cannot select another server.', + details: expect.objectContaining({ reason: 'managed-device-transport-mismatch' }), + }), + ); + expect(() => requireSameAndroidAdbServer(9_999, 15_037)).toThrowError( + expect.objectContaining({ details: { reason: 'managed-device-transport-mismatch' } }), + ); + }); +}); + +describe('lowerAndroidAdbInvocation', () => { + it('carries the owned server into argv and environment, and drops the option channel', () => { + const options: AndroidAdbExecutorOptions = { serverPort: 15_037, timeoutMs: 5_000 }; + const lowered = lowerAndroidAdbInvocation( + androidAdbInvocation(androidAdbSerialTarget('emulator-5554', 15_037), ['shell', 'id']), + options, + { ANDROID_ADB_SERVER_PORT: '5037', ADB_SERVER_SOCKET: 'tcp:elsewhere:5037' }, + ); + + expect(lowered.args).toEqual(['-P', '15037', '-s', 'emulator-5554', 'shell', 'id']); + expect(lowered.options).toEqual({ + timeoutMs: 5_000, + env: { + ANDROID_ADB_SERVER_PORT: '15037', + ADB_SERVER_SOCKET: undefined, + ANDROID_ADB_SERVER_ADDRESS: '127.0.0.1', + }, + }); + }); + + it('leaves an ambient request to the process environment it inherited', () => { + const options: AndroidAdbExecutorOptions = { timeoutMs: 5_000 }; + const lowered = lowerAndroidAdbInvocation( + parseAndroidAdbArgv(['-s', 'emulator-5554', 'shell', 'id']), + options, + { ANDROID_ADB_SERVER_PORT: '5037' }, + ); + + expect(lowered.args).toEqual(['-s', 'emulator-5554', 'shell', 'id']); + expect(lowered.options).toEqual({ timeoutMs: 5_000 }); + }); +}); + +describe('requireAndroidAdbServerPort', () => { + it('answers from the port the addressing owns', () => { + const invocation = androidAdbInvocation(androidAdbSerialTarget('A', 15_037), ['shell', 'id']); + expect(requireAndroidAdbServerPort(invocation, { serverPort: 15_037 })).toBe(15_037); + }); + + it('refuses a per-call port that would move an owned server', () => { + const invocation = androidAdbInvocation(androidAdbSerialTarget('A', 15_037), ['shell', 'id']); + expect(() => requireAndroidAdbServerPort(invocation, { serverPort: 9_999 })).toThrowError( + expect.objectContaining({ + code: 'COMMAND_FAILED', + details: expect.objectContaining({ reason: 'managed-device-transport-mismatch' }), + }), + ); + }); + + it('follows a per-call port while nothing owns one', () => { + const invocation = parseAndroidAdbArgv(['shell', 'id']); + expect(requireAndroidAdbServerPort(invocation, { serverPort: 9_999 })).toBe(9_999); + expect(requireAndroidAdbServerPort(invocation)).toBeUndefined(); + }); +}); + +describe('androidAdbHostTarget', () => { + it('addresses a server-level command without any device option', () => { + const invocation = androidAdbInvocation(androidAdbHostTarget(), [ + 'disconnect', + '127.0.0.1:5037', + ]); + expect(serializeAndroidAdbInvocation(invocation)).toEqual(['disconnect', '127.0.0.1:5037']); + }); + + it('is what an argv naming neither device nor server reads as', () => { + expect(parseAndroidAdbArgv(['devices']).target).toEqual(androidAdbHostTarget()); + }); +}); + +describe('serializeAndroidAdbInvocation', () => { + it('emits owned addressing ahead of the command in adb order', () => { + const invocation = androidAdbInvocation( + { + selector: { kind: 'serial', serial: 'A' }, + server: { kind: 'port', port: 5038 }, + waitFor: 'wait-for-device', + hostGlobals: ['-a'], + }, + ['shell', 'id'], + ); + expect(serializeAndroidAdbInvocation(invocation)).toEqual([ + '-P', + '5038', + '-s', + 'A', + '-a', + 'wait-for-device', + 'shell', + 'id', + ]); + }); + + it('appends a payload that looks like addressing without re-reading it', () => { + const command = ['-s', 'B', 'shell', 'id']; + const invocation = androidAdbInvocation(androidAdbSerialTarget('A'), command); + expect(serializeAndroidAdbInvocation(invocation)).toEqual([ + '-s', + 'A', + '-s', + 'B', + 'shell', + 'id', + ]); + expect(command).toEqual(['-s', 'B', 'shell', 'id']); + }); +}); + +describe('adoptAndroidAdbSerial', () => { + it('addresses an ambient target at the device it was built for', () => { + const target = adoptAndroidAdbSerial( + parseAndroidAdbArgv(['shell', 'getprop']).target, + 'emulator-5554', + ); + expect( + serializeAndroidAdbInvocation(androidAdbInvocation(target, ['shell', 'getprop'])), + ).toEqual(['-s', 'emulator-5554', 'shell', 'getprop']); + }); +}); + +describe('applyManagedAndroidAdbServer', () => { + it('rewrites addressing and carries the command by reference', () => { + const command = ['shell', 'dumpsys', 'window']; + const invocation = applyManagedAndroidAdbServer( + androidAdbInvocation(androidAdbSerialTarget('A'), command), + { port: 5039 }, + ); + expect(invocation.command).toBe(command); + expect(serializeAndroidAdbInvocation(invocation)).toEqual([ + '-P', + '5039', + '-s', + 'A', + 'shell', + 'dumpsys', + 'window', + ]); + }); + + it('refuses a port the caller typed for another server', () => { + expect(() => + applyManagedAndroidAdbServer(parseAndroidAdbArgv(['-P', '5037', '-s', 'A', 'shell', 'id']), { + port: 5039, + }), + ).toThrowError(/cannot select another server/); + }); + + it('accepts a port the caller typed for the server the lease already holds', () => { + const invocation = applyManagedAndroidAdbServer( + parseAndroidAdbArgv(['-P', '5039', '-s', 'A', 'shell', 'id']), + { port: 5039 }, + ); + expect(invocation.rawArgv).toBeUndefined(); + expect(serializeAndroidAdbInvocation(invocation)).toEqual([ + '-P', + '5039', + '-s', + 'A', + 'shell', + 'id', + ]); + }); + + it('refuses a host global the managed transport cannot restate', () => { + expect(() => + applyManagedAndroidAdbServer(parseAndroidAdbArgv(['-t', '42', 'shell', 'id']), { + port: 5039, + }), + ).toThrowError(/cannot select another target/); + }); + + it('refuses another device', () => { + expect(() => requireManagedAndroidAdbSerial(androidAdbSerialTarget('B'), 'A')).toThrowError( + /cannot address another device/, + ); + expect(requireManagedAndroidAdbSerial(androidAdbSerialTarget('A'), 'A').selector).toEqual({ + kind: 'serial', + serial: 'A', + }); + }); +}); + +describe('requireManagedAndroidAdbCommand', () => { + it('answers for the command behind any readiness token', () => { + requireManagedAndroidAdbCommand(['wait-for-device', 'shell', 'id']); + expect(() => requireManagedAndroidAdbCommand(['kill-server'])).toThrowError( + /cannot select another target/, + ); + expect(() => + requireManagedAndroidAdbCommand(['wait-for-recovery', 'kill-server']), + ).toThrowError(/cannot select another target/); + expect(() => requireManagedAndroidAdbCommand(['wait-for-magic', 'start-server'])).toThrowError( + /cannot select another target/, + ); + }); +}); + +describe('androidManagedAdbEnvironment', () => { + it('points a private server at loopback and clears a socket', () => { + const environment = androidManagedAdbEnvironment( + androidAdbSerialTarget('A', 5039), + { ANDROID_ADB_SERVER_PORT: '5037', ADB_SERVER_SOCKET: 'tcp:5037' }, + { ...process.env, PATH: '/bin' }, + ); + expect(environment?.ANDROID_ADB_SERVER_PORT).toBe('5039'); + expect(environment?.ANDROID_ADB_SERVER_ADDRESS).toBe('127.0.0.1'); + expect(environment?.ADB_SERVER_SOCKET).toBeUndefined(); + expect(environment?.PATH).toBe('/bin'); + }); + + it('leaves an ambient transport alone', () => { + const base = { ANDROID_ADB_SERVER_PORT: '5037' }; + expect(androidManagedAdbEnvironment(androidAdbSerialTarget('A'), {}, base)).toBe(base); + }); +}); diff --git a/packages/platform-android/src/adb-transport.ts b/packages/platform-android/src/adb-transport.ts index 37945ebfdf..976bcd095a 100644 --- a/packages/platform-android/src/adb-transport.ts +++ b/packages/platform-android/src/adb-transport.ts @@ -1,3 +1,4 @@ +import { AppError } from '@agent-device/kernel/errors'; import type { Readable, Stream, Writable } from 'node:stream'; import type { Rect } from '@agent-device/kernel/snapshot'; import type { @@ -6,9 +7,11 @@ import type { } from './helper-artifacts.ts'; import type { AndroidProviderTouchPlan } from './touch-plan-lowering.ts'; -// The adb transport vocabulary: the executor/provider shapes every module of the cluster (and -// the SDK, through the root shim) speaks, plus the one pure lowering from semantic install -// options to adb flags. No behavior lives here beyond that lowering. +// The adb transport vocabulary: the executor/provider shapes every module of the cluster (and the +// SDK, through the root shim) speaks, the pure lowering from semantic install options to adb +// flags, and the argv grammar that separates transport addressing — which device, which adb +// server — from the command that says what to run on it. Parsing, emitting, and the managed +// transport's rules over both live here so one declaration answers for every route. export type AndroidAdbExecutorOptions = { allowFailure?: boolean; @@ -31,10 +34,9 @@ export type AndroidAdbExecutorResult = { type AndroidAdbStdioOption = 'overlapped' | 'pipe' | 'ignore' | 'inherit'; /** - * A spawned adb process is long-lived — the snapshot helper session rides it for - * the whole session — so `timeoutMs` is not part of its options: background - * spawns arm no deadline, and a field that looked like one invited callers to - * kill their own helper. + * A spawned adb process is long-lived — the snapshot helper session rides it for the whole session + * — so `timeoutMs` is not part of its options: background spawns arm no deadline, and a field that + * looked like one invited callers to kill their own helper. */ export type AndroidAdbSpawnOptions = Omit & { cwd?: string; @@ -202,3 +204,457 @@ export function normalizeAndroidAdbInstallOptions(options?: AndroidAdbInstallOpt const { replace, ...execOptions } = options ?? {}; return { installArgs: replace ? ['-r'] : [], execOptions }; } + +/** + * Global options adb accepts before a command, mapped to the number of values each consumes. The + * grammar tables below are transcribed from `adb help` (adb 1.0.41, platform-tools 35.0.2): this + * arity, the `wait-for[-TRANSPORT]-STATE` product, and the server and transport commands a + * device-scoped call must never reach. + */ +export const ADB_GLOBAL_OPTIONS = { + '-a': 0, + '-d': 0, + '-e': 0, + '-s': 1, + '-t': 1, + '-H': 1, + '-P': 1, + '-L': 1, + '--one-device': 1, + '--exit-on-write-error': 0, +} as const satisfies Readonly>; + +type AndroidAdbGlobalOption = keyof typeof ADB_GLOBAL_OPTIONS; + +/** Transport qualifier of `wait-for[-TRANSPORT]-STATE`, spelled with its trailing separator. */ +export const ADB_WAIT_TRANSPORTS = ['', 'usb-', 'local-', 'any-'] as const; + +/** State argument of `wait-for[-TRANSPORT]-STATE`. */ +export const ADB_WAIT_STATES = [ + 'device', + 'recovery', + 'bootloader', + 'sideload', + 'sideload-window', +] as const; + +/** Tokens that address a server or a transport instead of running a device command. */ +export const ADB_MANAGED_FORBIDDEN_COMMANDS = [ + 'nodaemon', + 'server', + 'fork-server', + 'kill-server', + 'start-server', + 'connect', + 'disconnect', + 'reconnect', + 'attach', + 'detach', + 'pair', +] as const; + +export type AndroidAdbWait = + `wait-for-${(typeof ADB_WAIT_TRANSPORTS)[number]}${(typeof ADB_WAIT_STATES)[number]}`; + +const FORBIDDEN_COMMANDS = new Set(ADB_MANAGED_FORBIDDEN_COMMANDS); + +/** adb reads any `wait-for-` prefixed token as a readiness request, including ones it cannot parse. */ +function isAndroidAdbWaitToken(token: string): boolean { + return token.startsWith('wait-for-'); +} + +function parseAndroidAdbWaitToken(token: string): AndroidAdbWait | undefined { + if (!isAndroidAdbWaitToken(token)) return undefined; + const suffix = token.slice('wait-for-'.length); + return ADB_WAIT_TRANSPORTS.some( + (transport) => + suffix.startsWith(transport) && + (ADB_WAIT_STATES as readonly string[]).includes(suffix.slice(transport.length)), + ) + ? (token as AndroidAdbWait) + : undefined; +} + +function isAndroidAdbManagedForbiddenCommand(token: string): boolean { + return FORBIDDEN_COMMANDS.has(token); +} + +// Transport addressing separated from the device command: which device and which adb server a call +// is for, kept apart from the argv that says what to run on it. + +export type AndroidAdbSelector = + | Readonly<{ kind: 'unspecified' }> + | Readonly<{ kind: 'serial'; serial: string }>; + +export type AndroidAdbServer = + /** Whatever the ambient environment resolves: `$ANDROID_ADB_SERVER_ADDRESS`/`_PORT`, `-H`, `-L`. */ + Readonly<{ kind: 'ambient' }> | Readonly<{ kind: 'port'; port: number }>; + +export type AndroidAdbTarget = Readonly<{ + selector: AndroidAdbSelector; + server: AndroidAdbServer; + waitFor?: AndroidAdbWait; + /** Global options the typed grammar declines to own: `-a -d -e -t ID -H HOST -L SOCKET --…`. */ + hostGlobals?: readonly string[]; +}>; + +/** + * Addressing the caller owns, plus the opaque device command. `rawArgv` records the argv an + * invocation was parsed from so an unchanged, unmanaged invocation is handed to the process + * verbatim instead of being re-emitted from its parts. + */ +export type AndroidAdbInvocation = Readonly<{ + target: AndroidAdbTarget; + command: readonly string[]; + rawArgv?: readonly string[]; +}>; + +export function androidAdbSerialTarget( + serial: string, + serverPort?: number, +): Readonly { + return { + selector: { kind: 'serial', serial }, + server: serverPort === undefined ? { kind: 'ambient' } : { kind: 'port', port: serverPort }, + }; +} + +/** Addressing for a server-level command: it selects no device and names no private adb server. */ +export function androidAdbHostTarget(): Readonly { + return { selector: { kind: 'unspecified' }, server: { kind: 'ambient' } }; +} + +export function androidAdbInvocation( + target: AndroidAdbTarget, + command: readonly string[], + rawArgv?: readonly string[], +): AndroidAdbInvocation { + return { target, command, ...(rawArgv ? { rawArgv } : {}) }; +} + +/** Addresses `target` at `serial`, the device an ambient transport was built to answer for. */ +export function adoptAndroidAdbSerial( + target: AndroidAdbTarget, + serial: string, +): Readonly { + return { ...target, selector: { kind: 'serial', serial } }; +} + +/** + * The only place adb global options are emitted for an invocation whose addressing was rewritten. + * `command` is appended, never re-parsed. + */ +export function serializeAndroidAdbInvocation(invocation: AndroidAdbInvocation): string[] { + if (invocation.rawArgv) return [...invocation.rawArgv]; + const { target, command } = invocation; + const serialized: string[] = []; + if (target.server.kind === 'port') serialized.push('-P', String(target.server.port)); + if (target.selector.kind === 'serial') serialized.push('-s', target.selector.serial); + if (target.hostGlobals) serialized.push(...target.hostGlobals); + if (target.waitFor) serialized.push(target.waitFor); + serialized.push(...command); + return serialized; +} + +const OWNED_OPTIONS: Readonly> = { + '-s': 'serial', + '-P': 'server', +}; + +/** One option token read off the front of an argv, with the argv position it leaves behind. */ +type AndroidAdbOptionEffect = + | { kind: 'stop'; next: number } + | { kind: 'wait'; waitFor: AndroidAdbWait; next: number } + | { kind: 'unknown-wait'; token: string; next: number } + | { kind: 'serial'; serial: string; next: number } + | { kind: 'server'; port: number; next: number } + | { kind: 'global'; tokens: readonly string[]; next: number }; + +/** The addressing read off an argv so far, while the leading options are still being consumed. */ +type AndroidAdbAddressingRead = { + selector: AndroidAdbSelector; + server: AndroidAdbServer; + waitFor: AndroidAdbWait | undefined; + hostGlobals: string[]; +}; + +function readAndroidAdbOption(args: readonly string[], index: number): AndroidAdbOptionEffect { + const token = args[index]!; + const wait = parseAndroidAdbWaitToken(token); + if (wait) return { kind: 'wait', waitFor: wait, next: index + 1 }; + if (isAndroidAdbWaitToken(token)) return { kind: 'unknown-wait', token, next: index + 1 }; + const owned = OWNED_OPTIONS[token]; + if (owned === undefined) { + if (!token.startsWith('-')) return { kind: 'stop', next: index }; + const arity = ADB_GLOBAL_OPTIONS[token as AndroidAdbGlobalOption] ?? 0; + return { + kind: 'global', + tokens: [token, ...args.slice(index + 1, index + 1 + arity)], + next: index + 1 + arity, + }; + } + const value = args[index + 1] ?? ''; + if (owned === 'serial') return { kind: 'serial', serial: value, next: index + 2 }; + const port = Number(value); + return Number.isInteger(port) + ? { kind: 'server', port, next: index + 2 } + : { kind: 'global', tokens: [token, value], next: index + 2 }; +} + +function applyAndroidAdbOptionEffect( + effect: AndroidAdbOptionEffect, + read: AndroidAdbAddressingRead, +): void { + if (effect.kind === 'wait') { + // adb honors each `wait-for` token in turn, so a second one is addressing the grammar will + // not hold twice. It travels as a host global: verbatim on the ambient route, refused by the + // managed route rather than answered with one wait dropped. + if (read.waitFor === undefined) read.waitFor = effect.waitFor; + else read.hostGlobals.push(effect.waitFor); + } else if (effect.kind === 'unknown-wait') { + // adb waits for any `wait-for-` token, including one it cannot parse. The grammar cannot own a + // wait whose meaning it does not know, so it travels as a global: verbatim on the ambient + // route, refused by a managed one rather than answered by skipping the wait entirely. + read.hostGlobals.push(effect.token); + } else if (effect.kind === 'serial') { + // adb lets a later `-s` win. The first one is what the routers address by, so a second serial + // stays an unowned global and keeps its winning position on the way out — unless it names the + // same device, which asks for nothing the first one did not already say. + if (read.selector.kind === 'unspecified') + read.selector = { kind: 'serial', serial: effect.serial }; + else if (read.selector.serial !== effect.serial) read.hostGlobals.push('-s', effect.serial); + } else if (effect.kind === 'server') read.server = { kind: 'port', port: effect.port }; + else if (effect.kind === 'global') read.hostGlobals.push(...effect.tokens); +} + +/** + * Reads addressing out of a flat argv — the only direction that has to re-slice the payload. + * Never throws: an unowned or malformed global becomes `hostGlobals` for the policy layer to + * accept (ambient adb) or refuse (managed transport). + */ +export function parseAndroidAdbArgv(args: readonly string[]): AndroidAdbInvocation { + const host = androidAdbHostTarget(); + const read: AndroidAdbAddressingRead = { + selector: host.selector, + server: host.server, + waitFor: undefined, + hostGlobals: [], + }; + let index = 0; + for (;;) { + const effect: AndroidAdbOptionEffect = + index < args.length ? readAndroidAdbOption(args, index) : { kind: 'stop', next: index }; + if (effect.kind === 'stop') break; + applyAndroidAdbOptionEffect(effect, read); + index = effect.next; + } + const { selector, server, waitFor, hostGlobals } = read; + const target: AndroidAdbTarget = { + selector, + server, + ...(waitFor ? { waitFor } : {}), + ...(hostGlobals.length > 0 ? { hostGlobals } : {}), + }; + const requestedAddressing = + hostGlobals.length > 0 || + waitFor !== undefined || + selector.kind !== 'unspecified' || + server.kind === 'port'; + return { + target, + command: index === 0 ? args : args.slice(index), + rawArgv: requestedAddressing ? args : undefined, + }; +} + +/** + * The private-server port a managed transport owns for `invocation`, as opposed to a `-P` the + * caller typed into argv. Only `applyManagedAndroidAdbServer` output and hand-built targets carry + * owned addressing; anything still holding `rawArgv` keeps the caller's request in charge. + */ +function androidAdbOwnedServerPort(invocation: AndroidAdbInvocation): number | undefined { + if (invocation.rawArgv !== undefined) return undefined; + return invocation.target.server.kind === 'port' ? invocation.target.server.port : undefined; +} + +/** + * Reconciles the two channels an adb server port arrives on: the addressing this layer owns, and + * the per-call option an SDK caller can still pass. An owned port is not overridable — a caller + * cannot move a managed lease onto another adb server by naming one. + */ +export function requireAndroidAdbServerPort( + invocation: AndroidAdbInvocation, + options?: Pick, +): number | undefined { + return requireSameAndroidAdbServer(androidAdbOwnedServerPort(invocation), options?.serverPort); +} + +/** + * One adb server per request: whichever layer named a private server first, a second name for it + * that differs is a caller trying to move the transport elsewhere, not a choice to reconcile. + */ +export function requireSameAndroidAdbServer( + owned: number | undefined, + requested: number | undefined, +): number | undefined { + if (owned !== undefined && requested !== undefined && owned !== requested) { + throw transportMismatch('server'); + } + return owned ?? requested; +} + +/** + * The payload a device-scoped provider is asked to run: the caller's argv with this scope's own + * `-s` pair removed and everything else — readiness tokens, transport globals — left where the + * caller put it. Answers undefined when the argv addresses no device or another one. + */ +export function androidAdbPayloadWithoutSerial( + args: readonly string[], + serial: string, +): string[] | undefined { + let index = 0; + for (;;) { + const effect: AndroidAdbOptionEffect = + index < args.length ? readAndroidAdbOption(args, index) : { kind: 'stop', next: index }; + if (effect.kind === 'stop') return undefined; + if (effect.kind === 'serial') { + return effect.serial === serial + ? [...args.slice(0, index), ...args.slice(index + 2)] + : undefined; + } + index = effect.next; + } +} + +/** Managed (ADR 0021) transport: the lease owns a private adb server. */ +export type AndroidManagedAdbServer = Readonly<{ port: number }>; + +/** + * Adopts `invocation` for one managed adb server. The command travels by reference, so a payload + * authored upstream is the same array that reaches the spawn boundary. Addressing is rewritten + * wherever the lease adds something the caller left unsaid, and refused wherever the caller named + * a server of their own — see {@link requireManagedAndroidAdbAddressing}. + */ +export function applyManagedAndroidAdbServer( + invocation: AndroidAdbInvocation, + server: AndroidManagedAdbServer, +): AndroidAdbInvocation { + const { target, command } = invocation; + requireManagedAndroidAdbAddressing(target, server.port); + requireManagedAndroidAdbCommand(command); + return androidAdbInvocation( + { + selector: target.selector, + server: { kind: 'port', port: server.port }, + ...(target.waitFor ? { waitFor: target.waitFor } : {}), + }, + command, + ); +} + +/** Refuses a device selection that is not `serial`, and adopts an absent one, under a managed device. */ +export function requireManagedAndroidAdbSerial( + target: AndroidAdbTarget, + serial: string, +): Readonly { + requireUnconflictedAndroidAdbSelector(target.selector, serial); + return { ...target, selector: { kind: 'serial', serial } }; +} + +/** A selection naming another device is a conflict; a request that names none is not. */ +export function requireUnconflictedAndroidAdbSelector( + selector: AndroidAdbSelector, + serial: string, +): void { + if (selector.kind === 'serial' && selector.serial !== serial) throw transportMismatch('device'); +} + +/** + * What a managed transport may be addressed by: no target globals it cannot restate, and no adb + * server but the one it holds. + * + * A `-P` naming a different server is refused here, before anything is dispatched, rather than + * rewritten onto the lease's server. A caller who asked for 5037 and got 15038 would otherwise + * read a successful exit as evidence about 5037, which is the one answer a private server must not + * give. A `-P` naming this server, or none at all, is the ordinary case. + */ +export function requireManagedAndroidAdbAddressing( + target: AndroidAdbTarget, + managedPort: number, +): void { + if (target.hostGlobals) throw transportMismatch('target'); + requireSameAndroidAdbServer( + managedPort, + target.server.kind === 'port' ? target.server.port : undefined, + ); +} + +/** + * Server and transport lifecycle never belong to a device-scoped invocation. adb reads readiness + * tokens ahead of the real command, so the guard answers for the first token that is not one. + */ +export function requireManagedAndroidAdbCommand(command: readonly string[]): void { + const head = command.find((token) => !isAndroidAdbWaitToken(token)) ?? ''; + if (isAndroidAdbManagedForbiddenCommand(head)) throw transportMismatch('target'); +} + +/** What a managed transport refuses to be pointed at: another device, target, or adb server. */ +const TRANSPORT_MISMATCH_MESSAGES = { + device: 'Managed ADB transport cannot address another device.', + target: 'Managed ADB transport cannot select another target.', + server: 'Managed ADB transport cannot select another server.', +} as const; + +function transportMismatch(scope: keyof typeof TRANSPORT_MISMATCH_MESSAGES): never { + throw new AppError('COMMAND_FAILED', TRANSPORT_MISMATCH_MESSAGES[scope], { + reason: 'managed-device-transport-mismatch', + }); +} + +/** The per-call option an adb server port can arrive on, and nothing else. */ +type AndroidAdbServerOption = { + serverPort?: number; + env?: Record; +}; + +/** + * The flat argv and process options one adb request runs with: the server this layer owns, the + * environment that server implies, and the caller's own options with the port channel removed. + * Whoever spawns the process decides `detached`, because that is a question about the process. + */ +export function lowerAndroidAdbInvocation( + invocation: AndroidAdbInvocation, + options: Options | undefined, + environment: Record, +): { args: string[]; options: Omit } { + const { serverPort: _requestedServerPort, ...execOptions } = options ?? ({} as Options); + const port = requireAndroidAdbServerPort(invocation, options); + const resolved = + port === undefined ? invocation : applyManagedAndroidAdbServer(invocation, { port }); + const env = + port === undefined + ? undefined + : androidManagedAdbEnvironment(resolved.target, environment, execOptions.env); + return { + args: serializeAndroidAdbInvocation(resolved), + options: { ...execOptions, ...(env === undefined ? {} : { env }) }, + }; +} + +/** Process environment lowering for a private adb server; ambient invocations change nothing. */ +export function androidManagedAdbEnvironment( + target: AndroidAdbTarget, + environment: Record, + base?: Record, +): Record | undefined { + if (target.server.kind === 'port') { + return { + ...environment, + ...(base ?? {}), + ADB_SERVER_SOCKET: undefined, + ANDROID_ADB_SERVER_PORT: String(target.server.port), + ANDROID_ADB_SERVER_ADDRESS: '127.0.0.1', + }; + } + return base; +} diff --git a/packages/platform-android/src/device-boot.test.ts b/packages/platform-android/src/device-boot.test.ts index 526e368d6e..41fd98858e 100644 --- a/packages/platform-android/src/device-boot.test.ts +++ b/packages/platform-android/src/device-boot.test.ts @@ -15,10 +15,13 @@ const DEVICE: DeviceInfo = { const NOW_MS = 1_700_000_000_000; function answersUptime(stdout: string, exitCode = 0) { - let received: { serial: string; args: string[] } | undefined; + let received: { serial: string; args: readonly string[] } | undefined; bindAndroidAdbHostStub({ - execSerialAdb: async (serial, args) => { - received = { serial, args }; + execAdb: async (invocation) => { + if (invocation.target.selector.kind !== 'serial') { + throw new Error('expected a serial-target adb invocation'); + } + received = { serial: invocation.target.selector.serial, args: invocation.command }; return { exitCode, stdout, stderr: '' }; }, }); @@ -48,7 +51,7 @@ test('derives the boot instant from the uptime duration on the host clock', asyn test('a slow uptime answer cannot move the boot instant past the moment the probe began', async () => { bindAndroidAdbHostStub({ - execSerialAdb: async () => { + execAdb: async () => { vi.setSystemTime(NOW_MS + 4_000); return { exitCode: 0, stdout: '120.45 0', stderr: '' }; }, diff --git a/packages/platform-android/src/emulator-lifecycle.ts b/packages/platform-android/src/emulator-lifecycle.ts index d417381e97..74e36f6167 100644 --- a/packages/platform-android/src/emulator-lifecycle.ts +++ b/packages/platform-android/src/emulator-lifecycle.ts @@ -4,6 +4,7 @@ import { AppError, asAppError } from '@agent-device/kernel/errors'; import { type ExecResult, runCmdDetached, whichCmd } from '@agent-device/host-kit/command'; import { Deadline, retryWithPolicy, sleep } from '@agent-device/host-kit/retry'; +import { androidAdbInvocation, androidAdbSerialTarget } from './adb-transport.ts'; import { runAndroidHostAdb } from './adb-executor.ts'; import { bootFailureHint, classifyBootFailure } from '@agent-device/provision-kit/boot-diagnostics'; import { ensureAndroidSdkPathConfigured } from './sdk.ts'; @@ -154,11 +155,14 @@ async function readAndroidBootProp( timeoutMs = ANDROID_BOOT_PROP_TIMEOUT_MS, signal?: AbortSignal, ): Promise { - return await runAndroidHostAdb(['-s', serial, 'shell', 'getprop', 'sys.boot_completed'], { - allowFailure: true, - signal, - timeoutMs, - }); + return await runAndroidHostAdb( + androidAdbInvocation(androidAdbSerialTarget(serial), [ + 'shell', + 'getprop', + 'sys.boot_completed', + ]), + { allowFailure: true, signal, timeoutMs }, + ); } export async function waitForAndroidBoot( diff --git a/packages/platform-android/src/ime-lifecycle.test.ts b/packages/platform-android/src/ime-lifecycle.test.ts index 8577bb8c41..b6a20441b7 100644 --- a/packages/platform-android/src/ime-lifecycle.test.ts +++ b/packages/platform-android/src/ime-lifecycle.test.ts @@ -13,11 +13,11 @@ test('quick serial listing keeps an unbound adb host port loud', async () => { test('quick serial listing routes its global devices call through the scoped transport', async () => { bindAndroidAdbHostStub(); - const seenArgs: string[][] = []; + const seenArgs: Array = []; const serials = await withAndroidHostAdbTransport( - async (args) => { - seenArgs.push([...args]); + async (invocation) => { + seenArgs.push(invocation.command); return { stdout: 'List of devices attached\nemulator-5554\tdevice\n', stderr: '', diff --git a/packages/platform-android/src/ime-lifecycle.ts b/packages/platform-android/src/ime-lifecycle.ts index 2b12ddf36a..6386c6e8dc 100644 --- a/packages/platform-android/src/ime-lifecycle.ts +++ b/packages/platform-android/src/ime-lifecycle.ts @@ -1,3 +1,4 @@ +import { parseAndroidAdbArgv } from './adb-transport.ts'; import { requireAndroidAdbHost, runAndroidHostAdb } from './adb-host.ts'; export { @@ -18,7 +19,9 @@ export { export async function listAndroidAdbSerialsQuick(): Promise { requireAndroidAdbHost(); try { - const result = await runAndroidHostAdb(['devices'], { timeoutMs: 5_000 }); + const result = await runAndroidHostAdb(parseAndroidAdbArgv(['devices']), { + timeoutMs: 5_000, + }); return result.stdout .split('\n') .map((line) => line.trim()) diff --git a/packages/platform-android/src/mechanics.ts b/packages/platform-android/src/mechanics.ts index a7173e79f3..6338f3e992 100644 --- a/packages/platform-android/src/mechanics.ts +++ b/packages/platform-android/src/mechanics.ts @@ -5,6 +5,15 @@ * implementation seam for selected Android use; host-bound adb wiring is supplied separately by * the root composition module. */ +export { + androidAdbHostTarget, + androidAdbInvocation, + androidAdbSerialTarget, + lowerAndroidAdbInvocation, + parseAndroidAdbArgv, + serializeAndroidAdbInvocation, + type AndroidAdbInvocation, +} from './adb-transport.ts'; export { androidAdbResultError, attachAdbFailureHint, diff --git a/packages/platform-android/src/runtime.test.ts b/packages/platform-android/src/runtime.test.ts index 61adafcdda..65fdcf6c0b 100644 --- a/packages/platform-android/src/runtime.test.ts +++ b/packages/platform-android/src/runtime.test.ts @@ -7,6 +7,7 @@ import type { } from '@agent-device/contracts/platform-runtime-operations'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { createAndroidPlatformRuntime } from './runtime.ts'; +import type { AndroidAdbExecutorOptions, AndroidAdbInvocation } from './adb-transport.ts'; import { bindAndroidAdbHostStub } from './adb-host.fixtures.ts'; import { ANDROID_EMULATOR, @@ -27,20 +28,24 @@ test.each([ ['device', { ...ANDROID_EMULATOR, kind: 'device' as const }], ['unknown', UNKNOWN_KIND_DEVICE], ])('classifies the Android %s runtime denominator', async (_name, runtimeDevice) => { - const execSerialAdb = vi.fn(async (_serial: string, args: string[]) => { - if (args.includes('query-activities')) { - return { stdout: 'com.example.app/.MainActivity\n', stderr: '', exitCode: 0 }; - } - if (args.includes('dumpsys')) { - return { - stdout: 'mCurrentFocus=Window{1 u0 com.example.app/.MainActivity}', - stderr: '', - exitCode: 0, - }; - } - return { stdout: '', stderr: '', exitCode: 0 }; - }); - bindAndroidAdbHostStub({ execSerialAdb }); + const deviceCall = vi.fn( + async (_serial: string, args: string[], _options?: AndroidAdbExecutorOptions) => { + if (args.includes('query-activities')) { + return { stdout: 'com.example.app/.MainActivity\n', stderr: '', exitCode: 0 }; + } + if (args.includes('dumpsys')) { + return { + stdout: 'mCurrentFocus=Window{1 u0 com.example.app/.MainActivity}', + stderr: '', + exitCode: 0, + }; + } + return { stdout: '', stderr: '', exitCode: 0 }; + }, + ); + const execAdb = async (invocation: AndroidAdbInvocation, options?: AndroidAdbExecutorOptions) => + await deviceCall(deviceSerial(invocation), [...invocation.command], options); + bindAndroidAdbHostStub({ execAdb, spawnAdb: execAdb as never }); const host = androidRuntimeHost({ commands: { which: async () => 'tool', @@ -100,7 +105,7 @@ test.each([ await expect( binding.operations.listApps?.({ device: runtimeDevice, filter: 'all' }), ).resolves.toEqual([{ id: 'com.example.app', name: 'Example' }]); - expect(execSerialAdb).toHaveBeenCalledWith( + expect(deviceCall).toHaveBeenCalledWith( runtimeDevice.id, expect.arrayContaining(['query-activities']), expect.objectContaining({ allowFailure: true }), @@ -114,7 +119,7 @@ test.each([ package: 'com.example.app', activity: '.MainActivity', }); - expect(execSerialAdb).toHaveBeenCalledWith( + expect(deviceCall).toHaveBeenCalledWith( runtimeDevice.id, ['shell', 'dumpsys', 'window', 'windows'], expect.objectContaining({ allowFailure: true }), @@ -506,3 +511,8 @@ test('a host with no clipboard probe refuses rather than assuming support', asyn expect(facts.operations.readClipboard.available).toBe(false); expect(facts.operations.writeClipboard.available).toBe(false); }); + +function deviceSerial(invocation: AndroidAdbInvocation): string { + if (invocation.target.selector.kind !== 'serial') throw new Error('expected a serial target'); + return invocation.target.selector.serial; +} diff --git a/packages/provider-limrun/package.json b/packages/provider-limrun/package.json index 487d3b59df..e549f0e323 100644 --- a/packages/provider-limrun/package.json +++ b/packages/provider-limrun/package.json @@ -7,7 +7,8 @@ "dependencies": { "@agent-device/capture-kit": "workspace:*", "@agent-device/contracts": "workspace:*", - "@agent-device/kernel": "workspace:*" + "@agent-device/kernel": "workspace:*", + "@agent-device/platform-android": "workspace:*" }, "exports": { ".": { diff --git a/packages/provider-limrun/src/android.test.ts b/packages/provider-limrun/src/android.test.ts new file mode 100644 index 0000000000..17a5296a13 --- /dev/null +++ b/packages/provider-limrun/src/android.test.ts @@ -0,0 +1,50 @@ +import { expect, test, vi } from 'vitest'; +import { + androidAdbHostTarget, + androidAdbInvocation, + serializeAndroidAdbInvocation, + type AndroidAdbInvocation, +} from '@agent-device/platform-android/mechanics'; +import { cleanupLimrunAndroidAdbTunnel, type LimrunAndroidSession } from './android.ts'; +import type { LimrunAdbCommandOptions, LimrunAdbProvider } from './runtime-dependencies.ts'; + +const ok = { exitCode: 0, stdout: '', stderr: '' }; + +test('cleanup asks the platform to address the disconnect as a server-level command and hands it over unchanged', async () => { + const calls: Array<{ argv: string[]; options: LimrunAdbCommandOptions | undefined }> = []; + const close = vi.fn(); + const provider: LimrunAdbProvider = { exec: async () => ok }; + const hostAdbInvocation = vi.fn((command: readonly string[]) => + androidAdbInvocation(androidAdbHostTarget(), command), + ); + const session = { + platform: 'android', + adbProvider: provider, + adbSerial: '127.0.0.1:62001', + adbTunnel: { close }, + adbTunnelPromise: Promise.resolve(), + dependencies: { + android: { hostAdbInvocation }, + host: { + runAdb: async (invocation: AndroidAdbInvocation, options?: LimrunAdbCommandOptions) => { + calls.push({ argv: serializeAndroidAdbInvocation(invocation), options }); + return ok; + }, + }, + }, + } as unknown as LimrunAndroidSession; + + await cleanupLimrunAndroidAdbTunnel(session); + + expect(hostAdbInvocation).toHaveBeenCalledWith(['disconnect', '127.0.0.1:62001']); + expect(calls).toEqual([ + { + argv: ['disconnect', '127.0.0.1:62001'], + options: { allowFailure: true, timeoutMs: 10_000 }, + }, + ]); + expect(close).toHaveBeenCalledOnce(); + expect(session.adbSerial).toBeUndefined(); + expect(session.adbTunnel).toBeUndefined(); + expect(session.adbTunnelPromise).toBeUndefined(); +}); diff --git a/packages/provider-limrun/src/android.ts b/packages/provider-limrun/src/android.ts index e20a25eab6..66cc89748e 100644 --- a/packages/provider-limrun/src/android.ts +++ b/packages/provider-limrun/src/android.ts @@ -20,6 +20,7 @@ import type { LimrunPortReverseEndpoint, LimrunRuntimeDependencies, } from './runtime-dependencies.ts'; +import type { AndroidAdbInvocation } from '@agent-device/platform-android/mechanics'; import { normalizeOptionalString } from './strings.ts'; import { awaitLimrunDeploymentOperation, @@ -142,7 +143,7 @@ export async function cleanupLimrunAndroidAdbTunnel(session: LimrunAndroidSessio if (serial) { await cleanupAndroidPortReverse(session); await session.dependencies.host - .runAdb(['disconnect', serial], { + .runAdb(session.dependencies.android.hostAdbInvocation(['disconnect', serial]), { allowFailure: true, timeoutMs: 10_000, }) @@ -175,42 +176,48 @@ async function runLimrunAndroidAdb( args: string[], options?: LimrunAdbCommandOptions, ): Promise { - const { adbArgs, result } = await executeLimrunAndroidAdb(session, args, options); + const { invocation, result } = await executeLimrunAndroidAdb(session, args, options); return await requireSuccessfulLimrunAndroidAdb( - adbArgs, + invocation, result, options?.allowFailure, session.dependencies, ); } +/** + * The tunnel serial is this provider's own addressing decision: it goes on the invocation's + * target, so the command array the Android cluster handed over reaches the host unchanged. + */ async function executeLimrunAndroidAdb( session: LimrunAndroidAdbSession, args: string[], options?: LimrunAdbCommandOptions, -): Promise<{ adbArgs: string[]; result: LimrunAdbCommandResult }> { +): Promise<{ invocation: AndroidAdbInvocation; result: LimrunAdbCommandResult }> { const serial = await ensurePersistentAndroidAdbSerial(session); - const adbArgs = ['-s', serial, ...args]; - const result = await session.dependencies.host.runAdb(adbArgs, { + const invocation = session.dependencies.android.deviceAdbInvocation(serial, args); + const result = await session.dependencies.host.runAdb(invocation, { allowFailure: options?.allowFailure, binaryStdout: options?.binaryStdout, stdin: options?.stdin, timeoutMs: options?.timeoutMs ?? 30_000, signal: options?.signal, }); - return { adbArgs, result }; + return { invocation, result }; } async function requireSuccessfulLimrunAndroidAdb( - adbArgs: string[], + invocation: AndroidAdbInvocation, result: LimrunAdbCommandResult, allowFailure: boolean | undefined, dependencies: Pick, ): Promise { if (result.exitCode !== 0 && allowFailure !== true) { - throw await dependencies.android.adbError('Limrun Android ADB command failed', result, { - command: ['adb', ...adbArgs].join(' '), - }); + throw await dependencies.android.adbError( + 'Limrun Android ADB command failed', + result, + invocation, + ); } return result; } diff --git a/packages/provider-limrun/src/app-log-reconnect.test.ts b/packages/provider-limrun/src/app-log-reconnect.test.ts index aaa6311848..1c263e5119 100644 --- a/packages/provider-limrun/src/app-log-reconnect.test.ts +++ b/packages/provider-limrun/src/app-log-reconnect.test.ts @@ -4,8 +4,13 @@ const iosClient = vi.hoisted(() => ({ appLogTail: vi.fn(async () => 'provider line\n'), disconnect: vi.fn(), })); +type FakeAndroidTunnel = { + address: { address: string; port: number }; + close: () => void; +}; + const androidClient = vi.hoisted(() => ({ - startAdbTunnel: vi.fn(async () => { + startAdbTunnel: vi.fn(async (): Promise => { throw new Error('tunnel failed'); }), disconnect: vi.fn(), @@ -18,8 +23,19 @@ vi.mock('@limrun/api/instance-client', () => ({ createInstanceClient: vi.fn(async () => androidClient), })); +import { + androidAdbHostTarget, + androidAdbInvocation, + androidAdbSerialTarget, + serializeAndroidAdbInvocation, + type AndroidAdbInvocation, +} from '@agent-device/platform-android/mechanics'; import { reconnectLimrunAppLogReader } from './app-log-reconnect.ts'; -import type { LimrunRuntimeDependencies } from './runtime-dependencies.ts'; +import type { + LimrunAdbCommandOptions, + LimrunAdbExecutor, + LimrunRuntimeDependencies, +} from './runtime-dependencies.ts'; test('reattaches an owned Limrun instance without persisting credentials', async () => { const get = vi.fn(async () => ({ @@ -105,3 +121,91 @@ test('disconnects the Android instance client when tunnel acquisition fails', as ).rejects.toThrow('tunnel failed'); expect(androidClient.disconnect).toHaveBeenCalledOnce(); }); + +test('addresses app-log adb traffic at the tunnel serial and hands cleanup a command that selects no device', async () => { + const calls: Array<{ + selector: AndroidAdbInvocation['target']['selector']; + command: string[]; + argv: string[]; + options: LimrunAdbCommandOptions | undefined; + }> = []; + const closeTunnel = vi.fn(); + androidClient.startAdbTunnel.mockImplementationOnce(async () => ({ + address: { address: '127.0.0.1', port: 62_001 }, + close: closeTunnel, + })); + androidClient.disconnect.mockClear(); + const outcome = await reconnectLimrunAppLogReader({ + limrun: { + androidInstances: { + get: vi.fn(async () => ({ + metadata: { labels: { provider: 'limrun', leaseId: 'lease-a' } }, + status: { + state: 'ready', + apiUrl: 'https://instance', + adbWebSocketUrl: 'wss://adb', + token: 'secret', + }, + })), + }, + } as never, + descriptor: { + transport: 'limrun-log-poller', + platform: 'android', + leaseId: 'lease-a', + instanceId: 'instance-a', + appBundleId: 'com.example.app', + outputPath: '/sessions/one/app.log', + }, + dependencies: { + host: { + runAdb: async ( + invocation: AndroidAdbInvocation, + options?: LimrunAdbCommandOptions, + ): Promise<{ exitCode: number; stdout: string; stderr: string }> => { + calls.push({ + selector: invocation.target.selector, + command: [...invocation.command], + argv: serializeAndroidAdbInvocation(invocation), + options, + }); + return { exitCode: 0, stdout: '', stderr: '' }; + }, + }, + android: { + deviceAdbInvocation: (serial: string, command: readonly string[]) => + androidAdbInvocation(androidAdbSerialTarget(serial), command), + hostAdbInvocation: (command: readonly string[]) => + androidAdbInvocation(androidAdbHostTarget(), command), + readLogs: async (adb: LimrunAdbExecutor) => { + await adb(['shell', 'logcat', '-d', '-T', '20']); + return 'line\n'; + }, + }, + } as unknown as LimrunRuntimeDependencies, + }); + expect(outcome.status).toBe('opened'); + if (outcome.status !== 'opened') return; + + await outcome.reader.readLogs('com.example.app', 20); + // The tunnel serial is addressing, so the command the reader wrote travels unchanged: no `-s` + // is stitched back into the payload the provider hands over. + expect(calls[0]).toEqual({ + selector: { kind: 'serial', serial: '127.0.0.1:62001' }, + command: ['shell', 'logcat', '-d', '-T', '20'], + argv: ['-s', '127.0.0.1:62001', 'shell', 'logcat', '-d', '-T', '20'], + options: undefined, + }); + + await outcome.reader[Symbol.asyncDispose](); + // Ending the tunnel's adb connection is a server-level command, so it addresses no device and + // names the serial where adb expects it: in the command. + expect(calls[1]).toEqual({ + selector: { kind: 'unspecified' }, + command: ['disconnect', '127.0.0.1:62001'], + argv: ['disconnect', '127.0.0.1:62001'], + options: { allowFailure: true, timeoutMs: 10_000 }, + }); + expect(closeTunnel).toHaveBeenCalledOnce(); + expect(androidClient.disconnect).toHaveBeenCalledOnce(); +}); diff --git a/packages/provider-limrun/src/app-log-reconnect.ts b/packages/provider-limrun/src/app-log-reconnect.ts index a9fd2f2fd9..ab1f931d56 100644 --- a/packages/provider-limrun/src/app-log-reconnect.ts +++ b/packages/provider-limrun/src/app-log-reconnect.ts @@ -93,7 +93,11 @@ async function reconnectAndroid(options: { const adb = async ( args: string[], commandOptions?: Parameters[1], - ) => await options.dependencies.host.runAdb(['-s', serial, ...args], commandOptions); + ) => + await options.dependencies.host.runAdb( + options.dependencies.android.deviceAdbInvocation(serial, args), + commandOptions, + ); const reader: LimrunAppLogReader = { platform: 'android', leaseId: options.descriptor.leaseId, @@ -102,7 +106,10 @@ async function reconnectAndroid(options: { await options.dependencies.android.readLogs(adb, lineLimit), [Symbol.asyncDispose]: async () => { await options.dependencies.host - .runAdb(['disconnect', serial], { allowFailure: true, timeoutMs: 10_000 }) + .runAdb(options.dependencies.android.hostAdbInvocation(['disconnect', serial]), { + allowFailure: true, + timeoutMs: 10_000, + }) .catch(() => undefined); const results = await Promise.allSettled([ Promise.resolve().then(() => tunnel.close()), diff --git a/packages/provider-limrun/src/device-session.test.ts b/packages/provider-limrun/src/device-session.test.ts index 4601b8b761..0ec4220fb7 100644 --- a/packages/provider-limrun/src/device-session.test.ts +++ b/packages/provider-limrun/src/device-session.test.ts @@ -4,6 +4,11 @@ import type { AppsFilter, DeviceLease } from '@agent-device/contracts/device'; import type { Interactor } from '@agent-device/contracts/interactor-types'; import type { DeviceInfo } from '@agent-device/kernel/device'; import { AppError } from '@agent-device/kernel/errors'; +import { + androidAdbHostTarget, + androidAdbInvocation, + androidAdbSerialTarget, +} from '@agent-device/platform-android/mechanics'; import type { LimrunAdbExecutor, LimrunAdbProvider, @@ -44,6 +49,9 @@ const TEST_DEPENDENCIES = { dismissed: false, }), readLogs: async () => 'log line\n', + deviceAdbInvocation: (serial, command) => + androidAdbInvocation(androidAdbSerialTarget(serial), command), + hostAdbInvocation: (command) => androidAdbInvocation(androidAdbHostTarget(), command), adbError: async (message) => new AppError('COMMAND_FAILED', message), }, host: { diff --git a/packages/provider-limrun/src/runtime-dependencies.test.ts b/packages/provider-limrun/src/runtime-dependencies.test.ts index 8b69a8615b..ef0f423e44 100644 --- a/packages/provider-limrun/src/runtime-dependencies.test.ts +++ b/packages/provider-limrun/src/runtime-dependencies.test.ts @@ -3,6 +3,13 @@ import { test, vi } from 'vitest'; import type { AppsFilter, DeviceLease } from '@agent-device/contracts/device'; import type { Interactor } from '@agent-device/contracts/interactor-types'; import type { DeviceInfo } from '@agent-device/kernel/device'; +import { + androidAdbHostTarget, + androidAdbInvocation, + androidAdbSerialTarget, + serializeAndroidAdbInvocation, + type AndroidAdbInvocation, +} from '@agent-device/platform-android/mechanics'; import { AppError } from '@agent-device/kernel/errors'; import { createLimrunRuntime } from './runtime.ts'; import type { @@ -211,6 +218,52 @@ test('allocation rejects an unrelated foreground app after preinstall', async () assert.equal(fixture.getForegroundApp.mock.calls.length, 0); }); +test('a failed device adb command hands the addressed invocation to the root adapter', async () => { + const fixture = createContractFixture(); + const handed: Array = []; + const invocations: AndroidAdbInvocation[] = []; + const dependencies: LimrunRuntimeDependencies = { + ...fixture.dependencies, + android: { + ...fixture.dependencies.android, + adbError: async (message, _result, invocation) => { + handed.push(invocation); + return new AppError('COMMAND_FAILED', message); + }, + }, + host: { + ...fixture.dependencies.host, + runAdb: async (invocation) => { + invocations.push(invocation); + return { stdout: '', stderr: 'offline', exitCode: 1 }; + }, + }, + }; + const runtime = createLimrunRuntime({ apiKey: 'lim_test_key' }, dependencies); + + try { + await allocateAndroidDevice(runtime); + await assert.rejects(async () => + runtime.configurePortReverse?.({ + leaseId: 'lease-android', + devicePort: 8081, + hostPort: 8081, + name: 'metro', + }), + ); + } finally { + await runtime.shutdown(); + } + + // The provider restates no argv of its own: the failure carries the typed invocation it addressed. + const invocation = invocations[0]; + assert.ok(invocation); + assert.deepEqual(handed[0], invocation); + assert.deepEqual(invocation.target.server, { kind: 'ambient' }); + assert.deepEqual(invocation.target.selector, { kind: 'serial', serial: '127.0.0.1:62001' }); + assert.equal(serializeAndroidAdbInvocation(invocation)[0], '-s'); +}); + function createContractFixture() { const adbCalls: string[][] = []; const activeReverseMappings: LimrunPortReverseMapping[] = []; @@ -245,11 +298,15 @@ function createContractFixture() { dismissed: false, }), readLogs: async () => 'log line\n', + deviceAdbInvocation: (serial: string, command: readonly string[]) => + androidAdbInvocation(androidAdbSerialTarget(serial), command), + hostAdbInvocation: (command: readonly string[]) => + androidAdbInvocation(androidAdbHostTarget(), command), adbError: async (message: string) => new AppError('COMMAND_FAILED', message), }, host: { - runAdb: async (args: string[]) => { - adbCalls.push(args); + runAdb: async (invocation: AndroidAdbInvocation) => { + adbCalls.push(serializeAndroidAdbInvocation(invocation)); return { stdout: '', stderr: '', exitCode: 0 }; }, archiveDirectory: async () => undefined, diff --git a/packages/provider-limrun/src/runtime-dependencies.ts b/packages/provider-limrun/src/runtime-dependencies.ts index 1a51290d6f..d09e7e65f1 100644 --- a/packages/provider-limrun/src/runtime-dependencies.ts +++ b/packages/provider-limrun/src/runtime-dependencies.ts @@ -1,3 +1,4 @@ +import type { AndroidAdbInvocation } from '@agent-device/platform-android/mechanics'; import type { AppsFilter } from '@agent-device/contracts/device'; import type { Interactor } from '@agent-device/contracts/interactor-types'; import type { AndroidInputOwner } from '@agent-device/contracts/android-input-ownership'; @@ -88,15 +89,32 @@ export type LimrunAndroidRuntimeAdapter = { getKeyboardState(adb: LimrunAdbExecutor): Promise; dismissKeyboard(adb: LimrunAdbExecutor): Promise; readLogs(adb: LimrunAdbExecutor, lineLimit: number): Promise; + /** + * Addresses one device command at `serial`, the tunnel this provider opened. The serial is the + * provider's own addressing decision and belongs to the target, so the command array the Android + * cluster handed over reaches the host unchanged. The builders come from the composition root + * rather than an import: ADR-0019 keeps a provider's eager closure off the platform + * implementation, and an invocation is built by the platform's typed grammar. + */ + deviceAdbInvocation(serial: string, command: readonly string[]): AndroidAdbInvocation; + /** Addresses one server-level command, which selects no device. */ + hostAdbInvocation(command: readonly string[]): AndroidAdbInvocation; + /** + * Builds the failure an ADB command answered with. The invocation is what was asked of adb; how + * it is named in the error belongs to whoever renders it, not to this provider. + */ adbError( message: string, result: LimrunAdbCommandResult, - details?: Record, + invocation?: AndroidAdbInvocation, ): Promise; }; export type LimrunHostAdapter = { - runAdb(args: string[], options?: LimrunAdbCommandOptions): Promise; + runAdb( + invocation: AndroidAdbInvocation, + options?: LimrunAdbCommandOptions, + ): Promise; archiveDirectory(options: { sourceDirectory: string; entryName: string; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9d6d718254..0a7346fc35 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -437,6 +437,9 @@ importers: '@agent-device/kernel': specifier: workspace:* version: link:../kernel + '@agent-device/platform-android': + specifier: workspace:* + version: link:../platform-android devDependencies: '@limrun/api': specifier: ^0.49.3 diff --git a/scripts/layering/package-boundaries.test.ts b/scripts/layering/package-boundaries.test.ts index 9d65333ab4..04ce444a87 100644 --- a/scripts/layering/package-boundaries.test.ts +++ b/scripts/layering/package-boundaries.test.ts @@ -737,6 +737,7 @@ test('the real tree parses, declares, and passes R11', () => { '@agent-device/capture-kit', '@agent-device/contracts', '@agent-device/kernel', + '@agent-device/platform-android', ]); const rootExternalDependencies = rootExternalDependencyRanges(repoRoot); for (const pkg of packages) { diff --git a/src/managed-device-reachability.test.ts b/src/managed-device-reachability.test.ts index 7dd7629918..dbed3ae572 100644 --- a/src/managed-device-reachability.test.ts +++ b/src/managed-device-reachability.test.ts @@ -8,6 +8,7 @@ import type { } from '@agent-device/contracts/platform-runtime-host'; import { createAndroidInventoryModule } from '@agent-device/platform-android'; import { + parseAndroidAdbArgv, resolveAndroidAdbProvider, runAndroidHostAdb, } from '@agent-device/platform-android/mechanics'; @@ -87,7 +88,11 @@ test.skipIf(process.platform === 'win32')( adbPath, [ '#!/usr/bin/env node', + 'const fs = require("node:fs");', 'const args = process.argv.slice(2);', + 'const logPath = process.env.FAKE_ADB_CALL_LOG;', + 'if (logPath)', + String.raw` fs.appendFileSync(logPath, JSON.stringify({ args }) + "\n");`, 'const output = args.includes("devices") && args.includes("-l")', String.raw` ? "List of devices attached\nemulator-15037 device model:Managed_Pixel\n"`, ' : args.includes("ro.boot.qemu.avd_name")', @@ -104,9 +109,11 @@ test.skipIf(process.platform === 'win32')( ].join('\n'), ); fs.chmodSync(adbPath, 0o755); + const callLogPath = path.join(tmpDir, 'adb-calls.ndjson'); const previousPath = process.env.PATH; const previousPort = process.env.ANDROID_ADB_SERVER_PORT; process.env.PATH = `${tmpDir}${path.delimiter}${previousPath ?? ''}`; + process.env.FAKE_ADB_CALL_LOG = callLogPath; try { const reachability = createManagedLeaseReachability({ platform: 'android', @@ -142,18 +149,27 @@ test.skipIf(process.platform === 'win32')( progress: { report: () => {} }, } satisfies PlatformRequestScope, ); - const host = await runAndroidHostAdb(['devices']); - const hostWithWrongPort = await runAndroidHostAdb(['-P', '9999', 'devices']); + const host = await runAndroidHostAdb(parseAndroidAdbArgv(['devices'])); + // The lease owns the adb server, so a call naming another one is refused rather than + // answered on the lease's server: the caller would read it as evidence about 9999. + let wrongPortReason: unknown = 'not-refused'; + try { + await runAndroidHostAdb(parseAndroidAdbArgv(['-P', '9999', 'devices'])); + } catch (error) { + wrongPortReason = (error as { details?: { reason?: string } }).details?.reason; + } const provider = resolveAndroidAdbProvider(reachability.device); const serial = await provider.exec(['shell', 'id']); return { inventory, host: JSON.parse(host.stdout), - hostWithWrongPort: JSON.parse(hostWithWrongPort.stdout), + wrongPortReason, serial: JSON.parse(serial.stdout), }; }); - const outside = JSON.parse((await runAndroidHostAdb(['devices'])).stdout); + const outside = JSON.parse( + (await runAndroidHostAdb(parseAndroidAdbArgv(['devices']))).stdout, + ); expect(results).toEqual({ inventory: [ @@ -167,18 +183,23 @@ test.skipIf(process.platform === 'win32')( }, ], host: { args: ['-P', '15037', '-s', 'emulator-15037', 'devices'], port: '15037' }, - hostWithWrongPort: { - args: ['-P', '15037', '-s', 'emulator-15037', 'devices'], - port: '15037', - }, + wrongPortReason: 'managed-device-transport-mismatch', serial: { args: ['-P', '15037', '-s', 'emulator-15037', 'shell', 'id'], port: '15037', }, }); expect(outside).toEqual({ args: ['devices'], port: previousPort ?? null }); + // The refused request never reached adb: every call the lease dispatched named its own port. + const dispatched = fs + .readFileSync(callLogPath, 'utf8') + .split('\n') + .filter((line) => line !== '') + .map((line) => JSON.parse(line) as { args: string[] }); + expect(dispatched.filter((call) => call.args.includes('9999'))).toEqual([]); expect(process.env.ANDROID_ADB_SERVER_PORT).toBe(previousPort); } finally { + delete process.env.FAKE_ADB_CALL_LOG; if (previousPath === undefined) delete process.env.PATH; else process.env.PATH = previousPath; } diff --git a/src/platform-runtime-android-adb-host.test.ts b/src/platform-runtime-android-adb-host.test.ts index 1717c55988..2489eaed7f 100644 --- a/src/platform-runtime-android-adb-host.test.ts +++ b/src/platform-runtime-android-adb-host.test.ts @@ -5,6 +5,7 @@ import { test } from 'vitest'; import { AppError } from '@agent-device/kernel/errors'; import { createLocalAndroidAdbProvider, + parseAndroidAdbArgv, runAndroidHostAdb, } from '@agent-device/platform-android/mechanics'; import { ANDROID_EMULATOR } from './__tests__/test-utils/device-fixtures.ts'; @@ -49,7 +50,7 @@ test.skipIf(process.platform === 'win32')( await withFakeAdbOnPath( String.raw`process.stderr.write("error: device offline\n"); process.exit(1);`, async () => { - const error = await runAndroidHostAdb(['devices']).then( + const error = await runAndroidHostAdb(parseAndroidAdbArgv(['devices'])).then( () => assert.fail('expected local adb to reject'), (error: unknown) => error, ); @@ -69,9 +70,24 @@ test.skipIf(process.platform === 'win32')( const previousPort = process.env.ANDROID_ADB_SERVER_PORT; const previousSocket = process.env.ADB_SERVER_SOCKET; process.env.ADB_SERVER_SOCKET = 'tcp:inherited.example:9999'; + const callLogPath = path.join( + mkdtempForTestSync('agent-device-adb-call-log-'), + 'adb-calls.ndjson', + ); try { await withFakeAdbOnPath( - 'process.stdout.write(JSON.stringify({args: process.argv.slice(2), port: process.env.ANDROID_ADB_SERVER_PORT ?? null, address: process.env.ANDROID_ADB_SERVER_ADDRESS ?? null, socket: process.env.ADB_SERVER_SOCKET ?? null}));', + [ + 'const fs = require("node:fs");', + 'const call = {', + ' args: process.argv.slice(2),', + ' port: process.env.ANDROID_ADB_SERVER_PORT ?? null,', + ' address: process.env.ANDROID_ADB_SERVER_ADDRESS ?? null,', + ' socket: process.env.ADB_SERVER_SOCKET ?? null,', + '};', + 'const logPath = process.env.FAKE_ADB_CALL_LOG;', + String.raw`if (logPath) fs.appendFileSync(logPath, JSON.stringify(call) + "\n");`, + 'process.stdout.write(JSON.stringify(call));', + ].join('\n'), async () => { const provider = createLocalAndroidAdbProvider(ANDROID_EMULATOR, { serverPort: 15_037, @@ -81,8 +97,8 @@ test.skipIf(process.platform === 'win32')( args: string[]; port: string | null; }; - const serialWithWrongPort = JSON.parse( - (await adb(['-P', '9999', 'shell', 'id'])).stdout, + const serialOnItsOwnServer = JSON.parse( + (await adb(['-P', '15037', 'shell', 'id'])).stdout, ) as { args: string[]; port: string | null }; const serialWithWrongEnvironment = JSON.parse( ( @@ -96,8 +112,31 @@ test.skipIf(process.platform === 'win32')( ).stdout, ) as { args: string[]; port: string | null }; const host = JSON.parse( - (await runAndroidHostAdb(['-P', '9999', 'devices'], { serverPort: 15_038 })).stdout, + ( + await runAndroidHostAdb(parseAndroidAdbArgv(['-P', '15038', 'devices']), { + serverPort: 15_038, + }) + ).stdout, ) as { args: string[]; port: string | null }; + const ambientHost = JSON.parse( + (await runAndroidHostAdb(parseAndroidAdbArgv(['-P', '9999', 'devices']))).stdout, + ) as { args: string[]; port: string | null }; + // A private adb server is not a channel a caller may retarget, so a `-P` naming another + // one is refused before adb is asked anything. The port this route was built with is + // accepted however it is spelled, including as the first token of argv. + await assert.rejects(adb(['-P', '9999', 'shell', 'id']), { + details: { reason: 'managed-device-transport-mismatch' }, + }); + assert.throws(() => provider.spawn?.(['-P', '9999', 'logcat']), { + details: { reason: 'managed-device-transport-mismatch' }, + }); + // A host call names its own server, and the two ways of naming it have to agree. + await assert.rejects( + runAndroidHostAdb(parseAndroidAdbArgv(['-P', '9999', 'devices']), { + serverPort: 15_038, + }), + { details: { reason: 'managed-device-transport-mismatch' } }, + ); for (const selector of [ ['-H', 'foreign.example'], ['-L', 'tcp:foreign.example:5037'], @@ -135,7 +174,7 @@ test.skipIf(process.platform === 'win32')( address: '127.0.0.1', socket: null, }); - assert.deepEqual(serialWithWrongPort, serial); + assert.deepEqual(serialOnItsOwnServer, serial); assert.deepEqual(serialWithWrongEnvironment, serial); const waited = JSON.parse((await adb(['wait-for-device', 'shell', 'id'])).stdout); assert.deepEqual(waited, { @@ -148,9 +187,30 @@ test.skipIf(process.platform === 'win32')( address: '127.0.0.1', socket: null, }); + // Nothing named a private server on the ambient call, so its argv is what runs. + assert.deepEqual(ambientHost, { + args: ['-P', '9999', 'devices'], + port: null, + address: null, + socket: 'tcp:inherited.example:9999', + }); + // A refusal is before dispatch: the private route never asked adb for another server. An + // ambient host call naming 9999 is a different matter, and did run with that argv. + const dispatched = fs + .readFileSync(callLogPath, 'utf8') + .split('\n') + .filter((line) => line !== '') + .map((line) => JSON.parse(line) as { args: string[] }); + assert.deepEqual( + dispatched + .filter((call) => call.args.includes('-s')) + .filter((call) => call.args.includes('9999')), + [], + ); assert.equal(process.env.ANDROID_ADB_SERVER_PORT, previousPort); assert.equal(process.env.ADB_SERVER_SOCKET, 'tcp:inherited.example:9999'); }, + { FAKE_ADB_CALL_LOG: callLogPath }, ); } finally { if (previousSocket === undefined) delete process.env.ADB_SERVER_SOCKET; @@ -171,7 +231,7 @@ test.skipIf(process.platform === 'win32')( 'try { process.kill(-process.pid, 0); ownGroup = true; } catch {}', 'process.stdout.write(JSON.stringify({ ownGroup }));', ].join('\n'), - async () => await runAndroidHostAdb(['devices'], { timeoutMs: 3_000 }), + async () => await runAndroidHostAdb(parseAndroidAdbArgv(['devices']), { timeoutMs: 3_000 }), ); assert.equal((JSON.parse(reported.stdout) as { ownGroup: boolean }).ownGroup, true); diff --git a/src/platform-runtime-android-adb-host.ts b/src/platform-runtime-android-adb-host.ts index a26bfc57b9..96e222fdfe 100644 --- a/src/platform-runtime-android-adb-host.ts +++ b/src/platform-runtime-android-adb-host.ts @@ -1,6 +1,5 @@ import { bindAndroidAdbHost } from '@agent-device/platform-android/adb-host'; -import type { AndroidAdbExecutorOptions } from '@agent-device/platform-android/mechanics'; -import { AppError } from '@agent-device/kernel/errors'; +import { lowerAndroidAdbInvocation } from '@agent-device/platform-android/mechanics'; import { createHash, randomUUID } from 'node:crypto'; import { coerceExecResult, @@ -82,36 +81,25 @@ bindAndroidAdbHost({ }, writeBytes: async (filePath, value) => await writeFile(filePath, value), }, - execSerialAdb: async (serial, args, options) => { - const invocation = adbInvocation(['-s', serial, ...args], options); - return await withoutCommandExecutorOverride( - async () => - await runCmd('adb', invocation.args, { - ...invocation.options, - detached: process.platform !== 'win32', - }), - ); + execAdb: async (invocation, options) => { + const lowered = lowerAndroidAdbInvocation(invocation, options, environment); + return await runCmd('adb', lowered.args, { + ...lowered.options, + // adb's fork-server is a grandchild: without its own process group a deadline can only + // signal `adb` itself, and the server keeps the stdio pipes open behind it. + detached: process.platform !== 'win32', + }); }, - spawnSerialAdb: (serial, args, options) => { - const invocation = adbInvocation(['-s', serial, ...args], options); - const background = runCmdBackground('adb', invocation.args, { - ...invocation.options, + spawnAdb: (invocation, options) => { + const lowered = lowerAndroidAdbInvocation(invocation, options, environment); + const background = runCmdBackground('adb', lowered.args, { + ...lowered.options, allowFailure: true, captureOutput: false, }); void background.wait.catch(() => {}); return background.child; }, - execHostAdb: async (args, options) => { - const invocation = adbInvocation(args, options); - return await runCmd('adb', invocation.args, { - ...invocation.options, - // adb's fork-server is a grandchild: without its own process group a - // deadline can only signal `adb` itself, and the server keeps the stdio - // pipes open behind it. - detached: process.platform !== 'win32', - }); - }, withAdbCommandExecutorOverride: withCommandExecutorOverride, withoutAdbCommandExecutorOverride: withoutCommandExecutorOverride, coerceAdbResult: coerceExecResult, @@ -145,76 +133,3 @@ bindAndroidAdbHost({ return await makeEnsureAndroidHelperInstalled(config)(request); }, }); - -function adbInvocation( - args: string[], - options?: Options, -): { args: string[]; options: Omit } { - const { serverPort, ...withoutServerPort } = options ?? ({} as Options); - if (serverPort === undefined) return { args, options: withoutServerPort }; - return { - args: withServerPort(args, serverPort), - options: { - ...withoutServerPort, - env: { - ...environment, - ...(withoutServerPort.env ?? {}), - ADB_SERVER_SOCKET: undefined, - ANDROID_ADB_SERVER_PORT: String(serverPort), - ANDROID_ADB_SERVER_ADDRESS: '127.0.0.1', - }, - }, - }; -} - -function withServerPort(args: string[], serverPort: number): string[] { - const normalized = ['-P', String(serverPort)]; - let index = 0; - let serial: string | undefined; - while (index < args.length) { - const argument = args[index]; - if (argument === '-P') { - index += 2; - continue; - } - if (argument === '-s') { - if (serial !== undefined && serial !== args[index + 1]) throw transportMismatch(); - serial = args[index + 1]; - normalized.push(argument, args[index + 1]!); - index += 2; - continue; - } - if (argument?.startsWith('-')) throw transportMismatch(); - break; - } - const command = args.slice(index); - assertManagedAdbCommand(command); - return [...normalized, ...command]; -} - -function assertManagedAdbCommand(args: string[]): void { - const command = args.find((argument) => !argument.startsWith('wait-for-')); - if ( - [ - 'nodaemon', - 'server', - 'fork-server', - 'kill-server', - 'start-server', - 'connect', - 'disconnect', - 'reconnect', - 'attach', - 'detach', - 'pair', - ].includes(command ?? '') - ) { - throw transportMismatch(); - } -} - -function transportMismatch(): AppError { - return new AppError('COMMAND_FAILED', 'Managed ADB transport cannot select another target.', { - reason: 'managed-device-transport-mismatch', - }); -} diff --git a/src/platform-runtime-gateway.fixtures.ts b/src/platform-runtime-gateway.fixtures.ts index ec0882de56..fcba0c5186 100644 --- a/src/platform-runtime-gateway.fixtures.ts +++ b/src/platform-runtime-gateway.fixtures.ts @@ -316,6 +316,14 @@ export const limrunTestDependencies = { dismissed: false, }), readLogs: async () => '', + deviceAdbInvocation: (serial: string, command: readonly string[]) => ({ + target: { selector: { kind: 'serial', serial }, server: { kind: 'ambient' } }, + command, + }), + hostAdbInvocation: (command: readonly string[]) => ({ + target: { selector: { kind: 'unspecified' }, server: { kind: 'ambient' } }, + command, + }), adbError: async () => { throw new Error('unused'); }, diff --git a/src/sdk/limrun-runtime-dependencies.test.ts b/src/sdk/limrun-runtime-dependencies.test.ts index 26f2bf77c8..e670255780 100644 --- a/src/sdk/limrun-runtime-dependencies.test.ts +++ b/src/sdk/limrun-runtime-dependencies.test.ts @@ -136,26 +136,106 @@ test('Limrun appstate forwards an in-flight abort through the provider ADB execu assert.equal(observedSignal, controller.signal); }); -test('host.runAdb keeps its exported shape and routes through the host transport', async () => { +test('host.runAdb carries addressing apart from payload and routes through the host transport', async () => { const { createLimrunRuntimeDependencies } = await import('./limrun-runtime-dependencies.ts'); - const { withAndroidHostAdbTransport } = await import('@agent-device/platform-android/mechanics'); + const { + androidAdbInvocation, + androidAdbSerialTarget, + serializeAndroidAdbInvocation, + withAndroidHostAdbTransport, + } = await import('@agent-device/platform-android/mechanics'); const dependencies = createLimrunRuntimeDependencies(); - const seen: Array<{ args: string[]; options?: Record }> = []; + const seen: Array<{ + args: string[]; + payload: readonly string[]; + options?: Record; + }> = []; const result = await withAndroidHostAdbTransport( - async (args, options) => { - seen.push({ args, ...(options ? { options } : {}) }); + async (invocation, options) => { + seen.push({ + args: serializeAndroidAdbInvocation(invocation), + payload: invocation.command, + ...(options ? { options } : {}), + }); return { stdout: 'ok', stderr: '', exitCode: 0 }; }, async () => - await dependencies.host.runAdb(['disconnect', 'emulator-5554'], { - allowFailure: true, - timeoutMs: 10_000, - }), + await dependencies.host.runAdb( + androidAdbInvocation(androidAdbSerialTarget('emulator-5554'), [ + '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 } }, + { + args: ['-s', 'emulator-5554', 'disconnect', 'emulator-5554'], + payload: ['disconnect', 'emulator-5554'], + options: { allowFailure: true, timeoutMs: 10_000 }, + }, + ]); +}); + +test('adbError names the failed command with the platform serializer', async () => { + const { createLimrunRuntimeDependencies } = await import('./limrun-runtime-dependencies.ts'); + const { androidAdbInvocation, androidAdbSerialTarget } = + await import('@agent-device/platform-android/mechanics'); + const dependencies = createLimrunRuntimeDependencies(); + + const failure = await dependencies.android.adbError( + 'Limrun Android ADB command failed', + { exitCode: 1, stdout: '', stderr: 'device offline' }, + androidAdbInvocation(androidAdbSerialTarget('127.0.0.1:62001'), ['reverse', 'tcp:8081']), + ); + + assert.equal(failure.details?.command, 'adb -s 127.0.0.1:62001 reverse tcp:8081'); + + const addressless = await dependencies.android.adbError('ADB failed', { + exitCode: 1, + stdout: '', + stderr: 'no device', + }); + assert.equal(addressless.details?.command, undefined); +}); + +test('the adb invocation adapters address through the platform builders', async () => { + const { createLimrunRuntimeDependencies } = await import('./limrun-runtime-dependencies.ts'); + const { + androidAdbHostTarget, + androidAdbInvocation, + androidAdbSerialTarget, + serializeAndroidAdbInvocation, + } = await import('@agent-device/platform-android/mechanics'); + const dependencies = createLimrunRuntimeDependencies(); + + const device = dependencies.android.deviceAdbInvocation('127.0.0.1:62001', [ + 'shell', + 'pm', + 'list', + 'packages', ]); + assert.deepEqual( + device, + androidAdbInvocation(androidAdbSerialTarget('127.0.0.1:62001'), device.command), + ); + assert.deepEqual(device.command, ['shell', 'pm', 'list', 'packages']); + assert.deepEqual(serializeAndroidAdbInvocation(device), [ + '-s', + '127.0.0.1:62001', + 'shell', + 'pm', + 'list', + 'packages', + ]); + + const host = dependencies.android.hostAdbInvocation(['disconnect', '127.0.0.1:62001']); + assert.deepEqual(host, androidAdbInvocation(androidAdbHostTarget(), host.command)); + assert.deepEqual(serializeAndroidAdbInvocation(host), ['disconnect', '127.0.0.1:62001']); }); diff --git a/src/sdk/limrun-runtime-dependencies.ts b/src/sdk/limrun-runtime-dependencies.ts index 66d1a32896..0caf476cfa 100644 --- a/src/sdk/limrun-runtime-dependencies.ts +++ b/src/sdk/limrun-runtime-dependencies.ts @@ -4,7 +4,12 @@ import '../platform-runtime-android-adb-host.ts'; // ProviderDeviceRuntime.getInteractor is synchronous, so this factory is the deliberate static edge; // making it lazy would require a proxy interactor rather than this seam. import { createAndroidInteractor } from '../core/interactors/android.ts'; -import { runAndroidHostAdb } from '@agent-device/platform-android/mechanics'; +import { + androidAdbHostTarget, + androidAdbInvocation, + androidAdbSerialTarget, + runAndroidHostAdb, +} from '@agent-device/platform-android/mechanics'; import { execFailureDetails, runCmd } from '@agent-device/host-kit/command'; import { readVersion } from '@agent-device/host-kit/version'; @@ -57,14 +62,24 @@ export function createLimrunRuntimeDependencies(): LimrunRuntimeDependencies { timeoutMs: 5_000, }); }, - adbError: async (message, result, details) => { + deviceAdbInvocation: (serial, command) => + androidAdbInvocation(androidAdbSerialTarget(serial), command), + hostAdbInvocation: (command) => androidAdbInvocation(androidAdbHostTarget(), command), + adbError: async (message, result, invocation) => { // Error construction is async so the platform helper remains lazy until an ADB failure. - const { androidAdbResultError } = await import('@agent-device/platform-android/mechanics'); - return androidAdbResultError(message, result, details); + const { androidAdbResultError, serializeAndroidAdbInvocation } = + await import('@agent-device/platform-android/mechanics'); + return androidAdbResultError( + message, + result, + invocation + ? { command: `adb ${serializeAndroidAdbInvocation(invocation).join(' ')}` } + : undefined, + ); }, }, host: { - runAdb: async (args, options) => await runAndroidHostAdb(args, options), + runAdb: async (invocation, options) => await runAndroidHostAdb(invocation, options), archiveDirectory: async ({ sourceDirectory, entryName, archivePath }) => { const args = ['-qr', archivePath, entryName]; const result = await runCmd('zip', args, { diff --git a/test/integration/provider-scenarios/limrun-deployment-cancellation.fixtures.ts b/test/integration/provider-scenarios/limrun-deployment-cancellation.fixtures.ts index c535316dad..a88b19a104 100644 --- a/test/integration/provider-scenarios/limrun-deployment-cancellation.fixtures.ts +++ b/test/integration/provider-scenarios/limrun-deployment-cancellation.fixtures.ts @@ -120,6 +120,14 @@ function limrunDependencies(): LimrunRuntimeDependencies { dismissed: false, }), readLogs: async () => '', + deviceAdbInvocation: (serial: string, command: readonly string[]) => ({ + target: { selector: { kind: 'serial', serial }, server: { kind: 'ambient' } }, + command, + }), + hostAdbInvocation: (command: readonly string[]) => ({ + target: { selector: { kind: 'unspecified' }, server: { kind: 'ambient' } }, + command, + }), adbError: async (message) => new Error(message) as never, }, host: { diff --git a/test/integration/provider-scenarios/stale-provider-runtime-admission.test.ts b/test/integration/provider-scenarios/stale-provider-runtime-admission.test.ts index 699866d89c..8463ded663 100644 --- a/test/integration/provider-scenarios/stale-provider-runtime-admission.test.ts +++ b/test/integration/provider-scenarios/stale-provider-runtime-admission.test.ts @@ -269,6 +269,14 @@ function limrunDependencies(): LimrunRuntimeDependencies { dismissed: false, }), readLogs: async () => '', + deviceAdbInvocation: (serial: string, command: readonly string[]) => ({ + target: { selector: { kind: 'serial', serial }, server: { kind: 'ambient' } }, + command, + }), + hostAdbInvocation: (command: readonly string[]) => ({ + target: { selector: { kind: 'unspecified' }, server: { kind: 'ambient' } }, + command, + }), adbError: async (message) => new Error(message) as never, }, host: {