From 7be4220a3f1737a7b43ccb1798eefad9be8e19b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 15 Sep 2026 17:44:34 +0200 Subject: [PATCH 1/6] refactor(android): separate adb transport addressing from the command payload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One array carried two meanings: which device and which adb server a call was for, and what to run on it. Five relays read and rewrote the first meaning out of the second — findAdbSerialIndex, readAdbSerial, stripAdbSerialArgs, withServerPort, assertManagedAdbCommand — and a provider handed that array had to undo the stitching to recover the command its caller actually asked for. An invocation now carries the two apart: `{ target: { selector, server, waitFor?, hostGlobals? }, command, rawArgv? }`. One table holds the argv grammar — global option arity, the 20 `wait-for[-TRANSPORT][-STATE]` forms, the server and transport commands a device-scoped call must never reach — and one parse/serialize pair reads and writes it. `command` is appended and never re-parsed; a request nobody rewrote is emitted as the argv it arrived in. Four rules stop being implied: - a private adb server travels in the addressing that owns it, so a per-call `serverPort` cannot move a leased route onto another server; - under a private server, globals that transport cannot restate are refused rather than answered with addressing quietly left behind; - a forwarding provider receives the caller's argv with only the scope's own `-s` pair removed, so a readiness token survives the hop; - one typed refusal answers every mismatch, over device, target, or server. `provider-limrun` drops its structural copy of the invocation type and its own argv projection, and hands an ADB failure the invocation it addressed. The root host lowers a typed request in one function, which the test host now shares instead of imitating. Design trace and route table: #2617. Production: 12 files, +722/-265. --- .../src/__tests__/adb-executor.test.ts | 14 +- .../test-utils/android-host-test-setup.ts | 22 +- .../src/adb-executor-host.test.ts | 32 +- .../platform-android/src/adb-host.fixtures.ts | 11 +- packages/platform-android/src/adb-host.ts | 40 +- .../src/adb-provider-scope.test.ts | 303 ++++++++++-- .../src/adb-provider-scope.ts | 253 ++++++---- .../src/adb-transport.test.ts | 394 +++++++++++++++ .../platform-android/src/adb-transport.ts | 449 +++++++++++++++++- .../platform-android/src/device-boot.test.ts | 11 +- .../src/emulator-lifecycle.ts | 14 +- .../src/ime-lifecycle.test.ts | 6 +- .../platform-android/src/ime-lifecycle.ts | 5 +- packages/platform-android/src/mechanics.ts | 8 + packages/platform-android/src/runtime.test.ts | 42 +- packages/provider-limrun/package.json | 3 +- packages/provider-limrun/src/android.ts | 49 +- .../provider-limrun/src/app-log-reconnect.ts | 12 +- .../src/runtime-dependencies.test.ts | 54 ++- .../src/runtime-dependencies.ts | 12 +- pnpm-lock.yaml | 3 + scripts/layering/package-boundaries.test.ts | 1 + src/managed-device-reachability.test.ts | 11 +- src/platform-runtime-android-adb-host.test.ts | 11 +- src/platform-runtime-android-adb-host.ts | 111 +---- src/sdk/limrun-runtime-dependencies.test.ts | 65 ++- src/sdk/limrun-runtime-dependencies.ts | 15 +- 27 files changed, 1575 insertions(+), 376 deletions(-) create mode 100644 packages/platform-android/src/adb-transport.test.ts 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..eb987e401a 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,97 @@ 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([]); + + // 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. + await capture({ serial: DEVICE.id }, ['-t', '42', '-s', DEVICE.id, 'shell', 'ls']); + expect(providerCalls).toEqual([['-t', '42', '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 +234,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 +268,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 +332,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 +373,76 @@ 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([]); +}); + 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 +455,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 +463,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 +482,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 +530,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 +544,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 +564,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..8b4443aef2 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,79 @@ 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, so the + // call is refused rather than answered with addressing quietly left behind. Without a lease + // the caller's own adb invocation is what runs, as it always has. + if (scope.serverPort !== undefined) requireManagedAndroidAdbAddressing(invocation.target); + // 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..62e910bc03 --- /dev/null +++ b/packages/platform-android/src/adb-transport.test.ts @@ -0,0 +1,394 @@ +import { describe, expect, it } from 'vitest'; +import { + ADB_GLOBAL_OPTIONS, + ADB_MANAGED_FORBIDDEN_COMMANDS, + ADB_WAIT_STATES, + ADB_WAIT_TRANSPORTS, + 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('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('overwrites a port the caller typed', () => { + const invocation = applyManagedAndroidAdbServer( + parseAndroidAdbArgv(['-P', '5037', '-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..9d3d312d76 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,436 @@ 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 }, + }; +} + +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 read: AndroidAdbAddressingRead = { + selector: { kind: 'unspecified' }, + server: { kind: 'ambient' }, + 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. Addressing is rewritten; the command travels by + * reference, so a payload authored upstream is the same array that reaches the spawn boundary. + * A caller-supplied `-P` is overwritten, as it is today; anything else that preselects a target + * or server is refused, as it is today. + */ +export function applyManagedAndroidAdbServer( + invocation: AndroidAdbInvocation, + server: AndroidManagedAdbServer, +): AndroidAdbInvocation { + const { target, command } = invocation; + requireManagedAndroidAdbAddressing(target); + 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'); +} + +/** A caller cannot point a managed transport at another server or preselect another target. */ +export function requireManagedAndroidAdbAddressing(target: AndroidAdbTarget): void { + if (target.hostGlobals) throw transportMismatch('target'); +} + +/** + * 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..12a58d296d 100644 --- a/packages/platform-android/src/mechanics.ts +++ b/packages/platform-android/src/mechanics.ts @@ -5,6 +5,14 @@ * implementation seam for selected Android use; host-bound adb wiring is supplied separately by * the root composition module. */ +export { + 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.ts b/packages/provider-limrun/src/android.ts index e20a25eab6..364716b46b 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, @@ -136,13 +137,33 @@ export async function configureLimrunAndroidPortReverse( }); } +/** + * Addresses one command to the tunnel's persistent serial on whatever adb the host resolves. The + * serial is this provider's own addressing decision, so it belongs to the target and never to the + * command array the Android cluster handed over. + */ +export function limrunDeviceAdbInvocation(serial: string, args: string[]): AndroidAdbInvocation { + return { + target: { selector: { kind: 'serial', serial }, server: { kind: 'ambient' } }, + command: args, + }; +} + +/** Addresses one server-level command, which selects no device. */ +export function limrunHostAdbInvocation(args: string[]): AndroidAdbInvocation { + return { + target: { selector: { kind: 'unspecified' }, server: { kind: 'ambient' } }, + command: args, + }; +} + export async function cleanupLimrunAndroidAdbTunnel(session: LimrunAndroidSession): Promise { await session.adbTunnelPromise?.catch(() => {}); const serial = session.adbSerial; if (serial) { await cleanupAndroidPortReverse(session); await session.dependencies.host - .runAdb(['disconnect', serial], { + .runAdb(limrunHostAdbInvocation(['disconnect', serial]), { allowFailure: true, timeoutMs: 10_000, }) @@ -175,42 +196,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 = limrunDeviceAdbInvocation(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.ts b/packages/provider-limrun/src/app-log-reconnect.ts index a9fd2f2fd9..626bf0782d 100644 --- a/packages/provider-limrun/src/app-log-reconnect.ts +++ b/packages/provider-limrun/src/app-log-reconnect.ts @@ -7,6 +7,7 @@ import type { LimrunAppLogDescriptor } from './app-log-descriptor.ts'; import type { LimrunAppLogReader } from './app-log-poller.ts'; import type { LimrunAppLogReconnectOutcome } from './app-log-runtime.ts'; import type { LimrunRuntimeDependencies } from './runtime-dependencies.ts'; +import { limrunDeviceAdbInvocation, limrunHostAdbInvocation } from './android.ts'; export async function reconnectLimrunAppLogReader(options: { limrun: Limrun; @@ -93,7 +94,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( + limrunDeviceAdbInvocation(serial, args), + commandOptions, + ); const reader: LimrunAppLogReader = { platform: 'android', leaseId: options.descriptor.leaseId, @@ -102,7 +107,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(limrunHostAdbInvocation(['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/runtime-dependencies.test.ts b/packages/provider-limrun/src/runtime-dependencies.test.ts index 8b69a8615b..e552a51bdd 100644 --- a/packages/provider-limrun/src/runtime-dependencies.test.ts +++ b/packages/provider-limrun/src/runtime-dependencies.test.ts @@ -3,6 +3,10 @@ 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 { + 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 +215,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[] = []; @@ -248,8 +298,8 @@ function createContractFixture() { 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..5168099a90 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,22 @@ export type LimrunAndroidRuntimeAdapter = { getKeyboardState(adb: LimrunAdbExecutor): Promise; dismissKeyboard(adb: LimrunAdbExecutor): Promise; readLogs(adb: LimrunAdbExecutor, lineLimit: number): Promise; + /** + * 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..adb82de300 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'; @@ -142,8 +143,10 @@ 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'])); + const hostWithWrongPort = await runAndroidHostAdb( + parseAndroidAdbArgv(['-P', '9999', 'devices']), + ); const provider = resolveAndroidAdbProvider(reachability.device); const serial = await provider.exec(['shell', 'id']); return { @@ -153,7 +156,9 @@ test.skipIf(process.platform === 'win32')( 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: [ diff --git a/src/platform-runtime-android-adb-host.test.ts b/src/platform-runtime-android-adb-host.test.ts index 1717c55988..df6d0e6023 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, ); @@ -96,7 +97,11 @@ 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', '9999', 'devices']), { + serverPort: 15_038, + }) + ).stdout, ) as { args: string[]; port: string | null }; for (const selector of [ ['-H', 'foreign.example'], @@ -171,7 +176,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/sdk/limrun-runtime-dependencies.test.ts b/src/sdk/limrun-runtime-dependencies.test.ts index 26f2bf77c8..702a47a734 100644 --- a/src/sdk/limrun-runtime-dependencies.test.ts +++ b/src/sdk/limrun-runtime-dependencies.test.ts @@ -136,26 +136,71 @@ 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); +}); diff --git a/src/sdk/limrun-runtime-dependencies.ts b/src/sdk/limrun-runtime-dependencies.ts index 66d1a32896..883e30f622 100644 --- a/src/sdk/limrun-runtime-dependencies.ts +++ b/src/sdk/limrun-runtime-dependencies.ts @@ -57,14 +57,21 @@ export function createLimrunRuntimeDependencies(): LimrunRuntimeDependencies { timeoutMs: 5_000, }); }, - adbError: async (message, result, details) => { + 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, { From 39654f92a0d783c325e7bc92173f828b66390d90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 15 Sep 2026 19:02:25 +0200 Subject: [PATCH 2/6] refactor(android): refuse a caller-named adb server on a private transport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `-P` naming another server used to be rewritten onto the port the transport holds, so a caller who asked for 5037 could read a zero exit as an answer about 5037. One rule now covers every channel, argv or option: name no port and the transport uses its own; name this one and it is accepted; name another and the call is refused before dispatch. The provider-forwarding branch checks the parsed argv as well, because a provider receives argv and would hand the caller's `-P` to an adb it does not address. Ambient adb is untouched: with no private server named, the caller's argv is what runs. Each refusal is pinned at the process boundary — the fake adb records what it was asked, and the refused requests appear nowhere in that log. --- .../src/adb-provider-scope.test.ts | 34 ++++++++++ .../src/adb-provider-scope.ts | 11 ++-- .../src/adb-transport.test.ts | 12 +++- .../platform-android/src/adb-transport.ts | 29 +++++++-- src/managed-device-reachability.test.ts | 32 ++++++--- src/platform-runtime-android-adb-host.test.ts | 65 +++++++++++++++++-- 6 files changed, 157 insertions(+), 26 deletions(-) diff --git a/packages/platform-android/src/adb-provider-scope.test.ts b/packages/platform-android/src/adb-provider-scope.test.ts index eb987e401a..959a6114d2 100644 --- a/packages/platform-android/src/adb-provider-scope.test.ts +++ b/packages/platform-android/src/adb-provider-scope.test.ts @@ -172,10 +172,30 @@ test('a managed port scope refuses global options the provider cannot restate', ).rejects.toMatchObject({ details: { reason: 'managed-device-transport-mismatch' } }); expect(providerCalls).toEqual([]); + // A port typed into argv is the same conflict as one naming another target: the provider would + // hand the caller's `-P` to an adb it does not address, so the call is refused here. + providerCalls.length = 0; + await expect( + capture({ serial: DEVICE.id, serverPort: 15_037 }, [ + '-P', + '9_999', + '-s', + DEVICE.id, + 'shell', + 'ls', + ]), + ).rejects.toMatchObject({ details: { reason: 'managed-device-transport-mismatch' } }); + expect(providerCalls).toEqual([]); + // 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. await capture({ serial: DEVICE.id }, ['-t', '42', '-s', DEVICE.id, 'shell', 'ls']); expect(providerCalls).toEqual([['-t', '42', '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 () => { @@ -433,6 +453,20 @@ test('a device route answers for the server it was built with, not one a call na }, ); 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 () => { diff --git a/packages/platform-android/src/adb-provider-scope.ts b/packages/platform-android/src/adb-provider-scope.ts index 8b4443aef2..4c8ac4b28f 100644 --- a/packages/platform-android/src/adb-provider-scope.ts +++ b/packages/platform-android/src/adb-provider-scope.ts @@ -253,10 +253,13 @@ function createAndroidCommandExecutorOverride( 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, so the - // call is refused rather than answered with addressing quietly left behind. Without a lease - // the caller's own adb invocation is what runs, as it always has. - if (scope.serverPort !== undefined) requireManagedAndroidAdbAddressing(invocation.target); + // 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. diff --git a/packages/platform-android/src/adb-transport.test.ts b/packages/platform-android/src/adb-transport.test.ts index 62e910bc03..f1b603fc3f 100644 --- a/packages/platform-android/src/adb-transport.test.ts +++ b/packages/platform-android/src/adb-transport.test.ts @@ -324,9 +324,17 @@ describe('applyManagedAndroidAdbServer', () => { ]); }); - it('overwrites a port the caller typed', () => { + 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', '5037', '-s', 'A', 'shell', 'id']), + parseAndroidAdbArgv(['-P', '5039', '-s', 'A', 'shell', 'id']), { port: 5039 }, ); expect(invocation.rawArgv).toBeUndefined(); diff --git a/packages/platform-android/src/adb-transport.ts b/packages/platform-android/src/adb-transport.ts index 9d3d312d76..d24f336000 100644 --- a/packages/platform-android/src/adb-transport.ts +++ b/packages/platform-android/src/adb-transport.ts @@ -524,17 +524,17 @@ export function androidAdbPayloadWithoutSerial( export type AndroidManagedAdbServer = Readonly<{ port: number }>; /** - * Adopts `invocation` for one managed adb server. Addressing is rewritten; the command travels by - * reference, so a payload authored upstream is the same array that reaches the spawn boundary. - * A caller-supplied `-P` is overwritten, as it is today; anything else that preselects a target - * or server is refused, as it is today. + * 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); + requireManagedAndroidAdbAddressing(target, server.port); requireManagedAndroidAdbCommand(command); return androidAdbInvocation( { @@ -563,9 +563,24 @@ export function requireUnconflictedAndroidAdbSelector( if (selector.kind === 'serial' && selector.serial !== serial) throw transportMismatch('device'); } -/** A caller cannot point a managed transport at another server or preselect another target. */ -export function requireManagedAndroidAdbAddressing(target: AndroidAdbTarget): void { +/** + * 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, + ); } /** diff --git a/src/managed-device-reachability.test.ts b/src/managed-device-reachability.test.ts index adb82de300..dbed3ae572 100644 --- a/src/managed-device-reachability.test.ts +++ b/src/managed-device-reachability.test.ts @@ -88,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")', @@ -105,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', @@ -144,15 +150,20 @@ test.skipIf(process.platform === 'win32')( } satisfies PlatformRequestScope, ); const host = await runAndroidHostAdb(parseAndroidAdbArgv(['devices'])); - const hostWithWrongPort = await runAndroidHostAdb( - parseAndroidAdbArgv(['-P', '9999', '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), }; }); @@ -172,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 df6d0e6023..2489eaed7f 100644 --- a/src/platform-runtime-android-adb-host.test.ts +++ b/src/platform-runtime-android-adb-host.test.ts @@ -70,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, @@ -82,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( ( @@ -98,11 +113,30 @@ test.skipIf(process.platform === 'win32')( ) as { args: string[]; port: string | null }; const host = JSON.parse( ( - await runAndroidHostAdb(parseAndroidAdbArgv(['-P', '9999', 'devices']), { + 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'], @@ -140,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, { @@ -153,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; From 4d7a902199921780ac70e63057b7a667945cc1bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 15 Sep 2026 22:38:52 +0200 Subject: [PATCH 3/6] test(android): reach the server rule through the provider-forwarding branch A `-P` of `9_999` parses to no port at all, so the pair stayed an unowned global and the older host-global refusal answered the call before the server rule ever saw it: the assertion passed without reaching the check it was written to pin. The port now parses, the assertion names the server refusal, and a port naming the server this lease holds is pinned on the same branch as what the provider is handed. Removing the server check from `requireManagedAndroidAdbAddressing` now fails four tests across the provider branch, the device route, the reachability run, and the root host. --- .../src/adb-provider-scope.test.ts | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/platform-android/src/adb-provider-scope.test.ts b/packages/platform-android/src/adb-provider-scope.test.ts index 959a6114d2..ba81c80992 100644 --- a/packages/platform-android/src/adb-provider-scope.test.ts +++ b/packages/platform-android/src/adb-provider-scope.test.ts @@ -172,25 +172,37 @@ test('a managed port scope refuses global options the provider cannot restate', ).rejects.toMatchObject({ details: { reason: 'managed-device-transport-mismatch' } }); expect(providerCalls).toEqual([]); - // A port typed into argv is the same conflict as one naming another target: the provider would - // hand the caller's `-P` to an adb it does not address, so the call is refused here. + // 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', - '9_999', + '9999', '-s', DEVICE.id, 'shell', 'ls', ]), - ).rejects.toMatchObject({ details: { reason: 'managed-device-transport-mismatch' } }); + ).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. + // 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']); - expect(providerCalls).toEqual([['-t', '42', 'shell', 'ls']]); await capture({ serial: DEVICE.id }, ['-P', '9999', '-s', DEVICE.id, 'shell', 'ls']); expect(providerCalls).toEqual([ ['-t', '42', 'shell', 'ls'], From b246b9546c036eaff0575b3b3a11a9fce1da5483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 15 Sep 2026 22:38:52 +0200 Subject: [PATCH 4/6] docs(changelog): record the adb server refusal replacing the argv rewrite Names the three surfaces it reaches, and what does not change: a request naming no server or the server its route holds, and ambient adb. --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) 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. From 164a9fde6a63c9589289580e3b44185e7715a022 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 16 Sep 2026 12:22:11 +0200 Subject: [PATCH 5/6] test(limrun): pin the tunnel addressing the Android cluster hands its host port The live Limrun lane cannot run here, and these are the two call sites it would have validated, so the shapes it checks are pinned where they are decided. The tunnel serial rides on the invocation's target and never enters the command array the caller wrote; ending the tunnel is a server-level command that selects no device and names the serial where adb expects to read it; and reader cleanup leaves no serial addressable on the session. --- packages/provider-limrun/src/android.test.ts | 72 +++++++++++++ .../src/app-log-reconnect.test.ts | 101 +++++++++++++++++- 2 files changed, 171 insertions(+), 2 deletions(-) create mode 100644 packages/provider-limrun/src/android.test.ts diff --git a/packages/provider-limrun/src/android.test.ts b/packages/provider-limrun/src/android.test.ts new file mode 100644 index 0000000000..591366cd96 --- /dev/null +++ b/packages/provider-limrun/src/android.test.ts @@ -0,0 +1,72 @@ +import { expect, test, vi } from 'vitest'; +import { + serializeAndroidAdbInvocation, + type AndroidAdbInvocation, +} from '@agent-device/platform-android/mechanics'; +import { + cleanupLimrunAndroidAdbTunnel, + limrunDeviceAdbInvocation, + limrunHostAdbInvocation, + type LimrunAndroidSession, +} from './android.ts'; +import type { LimrunAdbCommandOptions, LimrunAdbProvider } from './runtime-dependencies.ts'; + +const ok = { exitCode: 0, stdout: '', stderr: '' }; + +test('carries the tunnel serial on the invocation target and never in the command', () => { + const device = limrunDeviceAdbInvocation('127.0.0.1:62001', ['shell', 'pm', 'list', 'packages']); + expect(device.target).toEqual({ + selector: { kind: 'serial', serial: '127.0.0.1:62001' }, + server: { kind: 'ambient' }, + }); + expect(device.command).toEqual(['shell', 'pm', 'list', 'packages']); + expect(device.rawArgv).toBeUndefined(); + expect(serializeAndroidAdbInvocation(device)).toEqual([ + '-s', + '127.0.0.1:62001', + 'shell', + 'pm', + 'list', + 'packages', + ]); +}); + +test('addresses a server-level command to no device', () => { + const host = limrunHostAdbInvocation(['disconnect', '127.0.0.1:62001']); + expect(host.target.selector).toEqual({ kind: 'unspecified' }); + expect(serializeAndroidAdbInvocation(host)).toEqual(['disconnect', '127.0.0.1:62001']); +}); + +test('cleanup disconnects the tunnel serial as a server-level command and drops it', async () => { + const calls: Array<{ argv: string[]; options: LimrunAdbCommandOptions | undefined }> = []; + const close = vi.fn(); + const provider: LimrunAdbProvider = { exec: async () => ok }; + const session = { + platform: 'android', + adbProvider: provider, + adbSerial: '127.0.0.1:62001', + adbTunnel: { close }, + adbTunnelPromise: Promise.resolve(), + dependencies: { + host: { + runAdb: async (invocation: AndroidAdbInvocation, options?: LimrunAdbCommandOptions) => { + calls.push({ argv: serializeAndroidAdbInvocation(invocation), options }); + return ok; + }, + }, + }, + } as unknown as LimrunAndroidSession; + + await cleanupLimrunAndroidAdbTunnel(session); + + 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/app-log-reconnect.test.ts b/packages/provider-limrun/src/app-log-reconnect.test.ts index aaa6311848..2b4fb4abe5 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,16 @@ vi.mock('@limrun/api/instance-client', () => ({ createInstanceClient: vi.fn(async () => androidClient), })); +import { + 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 +118,87 @@ 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: { + 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(); +}); From 0688fd0e7c5f0be65cb59aad56aade53ebafd029 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 16 Sep 2026 17:32:50 +0200 Subject: [PATCH 6/6] refactor(limrun): build adb invocations through the platform builders The provider wrote an AndroidAdbTarget by hand, so a field added to that type could drift without an error. The composition root now hands it deviceAdbInvocation and hostAdbInvocation, both built with androidAdbInvocation over androidAdbSerialTarget / the new androidAdbHostTarget, which is also what parseAndroidAdbArgv reads an unaddressed argv as. The provider keeps the addressing decision, not its syntax. It cannot import the mechanics facet itself: ADR-0019 keeps a provider's eager closure off concrete platform implementation, which eager-closure-budgets.test.ts reports as soon as the edge appears. --- .../src/adb-transport.test.ts | 15 +++++++ .../platform-android/src/adb-transport.ts | 10 ++++- packages/platform-android/src/mechanics.ts | 1 + packages/provider-limrun/src/android.test.ts | 40 +++++-------------- packages/provider-limrun/src/android.ts | 24 +---------- .../src/app-log-reconnect.test.ts | 7 ++++ .../provider-limrun/src/app-log-reconnect.ts | 5 +-- .../src/device-session.test.ts | 8 ++++ .../src/runtime-dependencies.test.ts | 7 ++++ .../src/runtime-dependencies.ts | 10 +++++ src/platform-runtime-gateway.fixtures.ts | 8 ++++ src/sdk/limrun-runtime-dependencies.test.ts | 35 ++++++++++++++++ src/sdk/limrun-runtime-dependencies.ts | 10 ++++- ...limrun-deployment-cancellation.fixtures.ts | 8 ++++ .../stale-provider-runtime-admission.test.ts | 8 ++++ 15 files changed, 137 insertions(+), 59 deletions(-) diff --git a/packages/platform-android/src/adb-transport.test.ts b/packages/platform-android/src/adb-transport.test.ts index f1b603fc3f..58fa3c34ab 100644 --- a/packages/platform-android/src/adb-transport.test.ts +++ b/packages/platform-android/src/adb-transport.test.ts @@ -4,6 +4,7 @@ import { ADB_MANAGED_FORBIDDEN_COMMANDS, ADB_WAIT_STATES, ADB_WAIT_TRANSPORTS, + androidAdbHostTarget, androidAdbInvocation, androidAdbPayloadWithoutSerial, androidAdbSerialTarget, @@ -255,6 +256,20 @@ describe('requireAndroidAdbServerPort', () => { }); }); +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( diff --git a/packages/platform-android/src/adb-transport.ts b/packages/platform-android/src/adb-transport.ts index d24f336000..976bcd095a 100644 --- a/packages/platform-android/src/adb-transport.ts +++ b/packages/platform-android/src/adb-transport.ts @@ -319,6 +319,11 @@ export function androidAdbSerialTarget( }; } +/** 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[], @@ -428,9 +433,10 @@ function applyAndroidAdbOptionEffect( * accept (ambient adb) or refuse (managed transport). */ export function parseAndroidAdbArgv(args: readonly string[]): AndroidAdbInvocation { + const host = androidAdbHostTarget(); const read: AndroidAdbAddressingRead = { - selector: { kind: 'unspecified' }, - server: { kind: 'ambient' }, + selector: host.selector, + server: host.server, waitFor: undefined, hostGlobals: [], }; diff --git a/packages/platform-android/src/mechanics.ts b/packages/platform-android/src/mechanics.ts index 12a58d296d..6338f3e992 100644 --- a/packages/platform-android/src/mechanics.ts +++ b/packages/platform-android/src/mechanics.ts @@ -6,6 +6,7 @@ * the root composition module. */ export { + androidAdbHostTarget, androidAdbInvocation, androidAdbSerialTarget, lowerAndroidAdbInvocation, diff --git a/packages/provider-limrun/src/android.test.ts b/packages/provider-limrun/src/android.test.ts index 591366cd96..17a5296a13 100644 --- a/packages/provider-limrun/src/android.test.ts +++ b/packages/provider-limrun/src/android.test.ts @@ -1,46 +1,22 @@ import { expect, test, vi } from 'vitest'; import { + androidAdbHostTarget, + androidAdbInvocation, serializeAndroidAdbInvocation, type AndroidAdbInvocation, } from '@agent-device/platform-android/mechanics'; -import { - cleanupLimrunAndroidAdbTunnel, - limrunDeviceAdbInvocation, - limrunHostAdbInvocation, - type LimrunAndroidSession, -} from './android.ts'; +import { cleanupLimrunAndroidAdbTunnel, type LimrunAndroidSession } from './android.ts'; import type { LimrunAdbCommandOptions, LimrunAdbProvider } from './runtime-dependencies.ts'; const ok = { exitCode: 0, stdout: '', stderr: '' }; -test('carries the tunnel serial on the invocation target and never in the command', () => { - const device = limrunDeviceAdbInvocation('127.0.0.1:62001', ['shell', 'pm', 'list', 'packages']); - expect(device.target).toEqual({ - selector: { kind: 'serial', serial: '127.0.0.1:62001' }, - server: { kind: 'ambient' }, - }); - expect(device.command).toEqual(['shell', 'pm', 'list', 'packages']); - expect(device.rawArgv).toBeUndefined(); - expect(serializeAndroidAdbInvocation(device)).toEqual([ - '-s', - '127.0.0.1:62001', - 'shell', - 'pm', - 'list', - 'packages', - ]); -}); - -test('addresses a server-level command to no device', () => { - const host = limrunHostAdbInvocation(['disconnect', '127.0.0.1:62001']); - expect(host.target.selector).toEqual({ kind: 'unspecified' }); - expect(serializeAndroidAdbInvocation(host)).toEqual(['disconnect', '127.0.0.1:62001']); -}); - -test('cleanup disconnects the tunnel serial as a server-level command and drops it', async () => { +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, @@ -48,6 +24,7 @@ test('cleanup disconnects the tunnel serial as a server-level command and drops adbTunnel: { close }, adbTunnelPromise: Promise.resolve(), dependencies: { + android: { hostAdbInvocation }, host: { runAdb: async (invocation: AndroidAdbInvocation, options?: LimrunAdbCommandOptions) => { calls.push({ argv: serializeAndroidAdbInvocation(invocation), options }); @@ -59,6 +36,7 @@ test('cleanup disconnects the tunnel serial as a server-level command and drops await cleanupLimrunAndroidAdbTunnel(session); + expect(hostAdbInvocation).toHaveBeenCalledWith(['disconnect', '127.0.0.1:62001']); expect(calls).toEqual([ { argv: ['disconnect', '127.0.0.1:62001'], diff --git a/packages/provider-limrun/src/android.ts b/packages/provider-limrun/src/android.ts index 364716b46b..66cc89748e 100644 --- a/packages/provider-limrun/src/android.ts +++ b/packages/provider-limrun/src/android.ts @@ -137,33 +137,13 @@ export async function configureLimrunAndroidPortReverse( }); } -/** - * Addresses one command to the tunnel's persistent serial on whatever adb the host resolves. The - * serial is this provider's own addressing decision, so it belongs to the target and never to the - * command array the Android cluster handed over. - */ -export function limrunDeviceAdbInvocation(serial: string, args: string[]): AndroidAdbInvocation { - return { - target: { selector: { kind: 'serial', serial }, server: { kind: 'ambient' } }, - command: args, - }; -} - -/** Addresses one server-level command, which selects no device. */ -export function limrunHostAdbInvocation(args: string[]): AndroidAdbInvocation { - return { - target: { selector: { kind: 'unspecified' }, server: { kind: 'ambient' } }, - command: args, - }; -} - export async function cleanupLimrunAndroidAdbTunnel(session: LimrunAndroidSession): Promise { await session.adbTunnelPromise?.catch(() => {}); const serial = session.adbSerial; if (serial) { await cleanupAndroidPortReverse(session); await session.dependencies.host - .runAdb(limrunHostAdbInvocation(['disconnect', serial]), { + .runAdb(session.dependencies.android.hostAdbInvocation(['disconnect', serial]), { allowFailure: true, timeoutMs: 10_000, }) @@ -215,7 +195,7 @@ async function executeLimrunAndroidAdb( options?: LimrunAdbCommandOptions, ): Promise<{ invocation: AndroidAdbInvocation; result: LimrunAdbCommandResult }> { const serial = await ensurePersistentAndroidAdbSerial(session); - const invocation = limrunDeviceAdbInvocation(serial, args); + const invocation = session.dependencies.android.deviceAdbInvocation(serial, args); const result = await session.dependencies.host.runAdb(invocation, { allowFailure: options?.allowFailure, binaryStdout: options?.binaryStdout, diff --git a/packages/provider-limrun/src/app-log-reconnect.test.ts b/packages/provider-limrun/src/app-log-reconnect.test.ts index 2b4fb4abe5..1c263e5119 100644 --- a/packages/provider-limrun/src/app-log-reconnect.test.ts +++ b/packages/provider-limrun/src/app-log-reconnect.test.ts @@ -24,6 +24,9 @@ vi.mock('@limrun/api/instance-client', () => ({ })); import { + androidAdbHostTarget, + androidAdbInvocation, + androidAdbSerialTarget, serializeAndroidAdbInvocation, type AndroidAdbInvocation, } from '@agent-device/platform-android/mechanics'; @@ -170,6 +173,10 @@ test('addresses app-log adb traffic at the tunnel serial and hands cleanup a com }, }, 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'; diff --git a/packages/provider-limrun/src/app-log-reconnect.ts b/packages/provider-limrun/src/app-log-reconnect.ts index 626bf0782d..ab1f931d56 100644 --- a/packages/provider-limrun/src/app-log-reconnect.ts +++ b/packages/provider-limrun/src/app-log-reconnect.ts @@ -7,7 +7,6 @@ import type { LimrunAppLogDescriptor } from './app-log-descriptor.ts'; import type { LimrunAppLogReader } from './app-log-poller.ts'; import type { LimrunAppLogReconnectOutcome } from './app-log-runtime.ts'; import type { LimrunRuntimeDependencies } from './runtime-dependencies.ts'; -import { limrunDeviceAdbInvocation, limrunHostAdbInvocation } from './android.ts'; export async function reconnectLimrunAppLogReader(options: { limrun: Limrun; @@ -96,7 +95,7 @@ async function reconnectAndroid(options: { commandOptions?: Parameters[1], ) => await options.dependencies.host.runAdb( - limrunDeviceAdbInvocation(serial, args), + options.dependencies.android.deviceAdbInvocation(serial, args), commandOptions, ); const reader: LimrunAppLogReader = { @@ -107,7 +106,7 @@ async function reconnectAndroid(options: { await options.dependencies.android.readLogs(adb, lineLimit), [Symbol.asyncDispose]: async () => { await options.dependencies.host - .runAdb(limrunHostAdbInvocation(['disconnect', serial]), { + .runAdb(options.dependencies.android.hostAdbInvocation(['disconnect', serial]), { allowFailure: true, timeoutMs: 10_000, }) 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 e552a51bdd..ef0f423e44 100644 --- a/packages/provider-limrun/src/runtime-dependencies.test.ts +++ b/packages/provider-limrun/src/runtime-dependencies.test.ts @@ -4,6 +4,9 @@ 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'; @@ -295,6 +298,10 @@ 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: { diff --git a/packages/provider-limrun/src/runtime-dependencies.ts b/packages/provider-limrun/src/runtime-dependencies.ts index 5168099a90..d09e7e65f1 100644 --- a/packages/provider-limrun/src/runtime-dependencies.ts +++ b/packages/provider-limrun/src/runtime-dependencies.ts @@ -89,6 +89,16 @@ 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. 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 702a47a734..e670255780 100644 --- a/src/sdk/limrun-runtime-dependencies.test.ts +++ b/src/sdk/limrun-runtime-dependencies.test.ts @@ -204,3 +204,38 @@ test('adbError names the failed command with the platform serializer', async () }); 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 883e30f622..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,6 +62,9 @@ export function createLimrunRuntimeDependencies(): LimrunRuntimeDependencies { timeoutMs: 5_000, }); }, + 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, serializeAndroidAdbInvocation } = 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: {