From daa08b1718a8947207e0a809ec1e7ae3fba0b012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 08:20:02 +0200 Subject: [PATCH 1/5] fix(android): add a no-rebind mode to the port-reverse provider createExecAndroidPortReverseProvider and createAndroidPortReverseManager (executor form) take { noRebind }. The provider then runs adb reverse --no-rebind for an endpoint it did not bind, still rebinds its own mappings, and fails a refusal with COMMAND_FAILED and details.reason 'android_port_reverse_rebind_refused'. The refusal is classified from adb reverse --list, not from adb's stderr. The localhost URL auto-reverse keeps that reason on the error it rethrows. --- .../src/__tests__/app-lifecycle-open.test.ts | 37 +++++ .../src/adb-port-reverse.test.ts | 75 ++++++++++ .../platform-android/src/adb-port-reverse.ts | 139 ++++++++++++------ .../platform-android/src/app-lifecycle.ts | 1 + website/docs/docs/client-api.md | 7 +- 5 files changed, 211 insertions(+), 48 deletions(-) diff --git a/packages/platform-android/src/__tests__/app-lifecycle-open.test.ts b/packages/platform-android/src/__tests__/app-lifecycle-open.test.ts index 1abced327e..d4734d204a 100644 --- a/packages/platform-android/src/__tests__/app-lifecycle-open.test.ts +++ b/packages/platform-android/src/__tests__/app-lifecycle-open.test.ts @@ -210,6 +210,43 @@ test('openAndroidApp ensures Android reverse before localhost deep link launch', ]); }); +test('openAndroidApp keeps the typed reason of a refused localhost reverse', async () => { + const device: DeviceInfo = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator', + booted: true, + }; + const launches: (readonly string[])[] = []; + + await assert.rejects( + () => + withAndroidAdbProvider( + { + exec: async (args) => { + launches.push(args); + return { stdout: '', stderr: '', exitCode: 0 }; + }, + reverse: { + ensure: async () => { + throw new AppError('COMMAND_FAILED', 'already mapped', { + reason: 'android_port_reverse_rebind_refused', + }); + }, + remove: async () => {}, + removeAllOwned: async () => {}, + }, + }, + { serial: 'emulator-5554' }, + async () => await openAndroidApp(device, 'exp://127.0.0.1:8081'), + ), + (error: unknown) => + error instanceof AppError && error.details?.reason === 'android_port_reverse_rebind_refused', + ); + assert.deepEqual(launches, []); +}); + test('openAndroidApp ensures Android reverse before localhost app-bound deep link launch', async () => { const device: DeviceInfo = { platform: 'android', diff --git a/packages/platform-android/src/adb-port-reverse.test.ts b/packages/platform-android/src/adb-port-reverse.test.ts index 24d93bfd71..66f4b51244 100644 --- a/packages/platform-android/src/adb-port-reverse.test.ts +++ b/packages/platform-android/src/adb-port-reverse.test.ts @@ -67,3 +67,78 @@ test('a provider-owned reverse implementation is reused as-is when already manag const second = createAndroidPortReverseManager({ exec: async () => ok(), reverse: first }); expect(second).toBe(first); }); + +test('no-rebind refuses a device mapping another client owns with a typed reason', async () => { + bindAndroidAdbHostStub(); + const calls: (readonly string[])[] = []; + const manager = createAndroidPortReverseManager( + async (args) => { + calls.push(args); + if (args.includes('--no-rebind')) { + return { exitCode: 1, stdout: '', stderr: 'adb: error: cannot rebind existing socket' }; + } + return ok(args[1] === '--list' ? 'owner-host tcp:8081 tcp:8081\n' : ''); + }, + { noRebind: true }, + ); + + await expect( + manager.ensure({ local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' }), + ).rejects.toMatchObject({ + code: 'COMMAND_FAILED', + details: { + reason: 'android_port_reverse_rebind_refused', + existing: { local: 'tcp:8081', remote: 'tcp:8081' }, + }, + }); + await manager.removeAllOwned('metro'); + + expect(calls).toEqual([ + ['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081'], + ['reverse', '--list'], + ]); +}); + +test('no-rebind still rebinds a mapping the same provider created', async () => { + bindAndroidAdbHostStub(); + const calls: (readonly string[])[] = []; + const manager = createAndroidPortReverseManager( + async (args) => { + calls.push(args); + return ok(); + }, + { noRebind: true }, + ); + + await manager.ensure({ local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' }); + await manager.ensure({ local: 'tcp:8081', remote: 'tcp:9090', ownerId: 'metro' }); + + expect(calls).toEqual([ + ['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081'], + ['reverse', 'tcp:8081', 'tcp:9090'], + ]); +}); + +test('no-rebind reports an adb failure when the device lists no mapping for the endpoint', async () => { + bindAndroidAdbHostStub(); + const manager = createAndroidPortReverseManager( + async (args) => + args.includes('--no-rebind') + ? { exitCode: 1, stdout: '', stderr: 'error: device offline' } + : ok(), + { noRebind: true }, + ); + + const failure = await manager + .ensure({ local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' }) + .then( + () => undefined, + (error: unknown) => error, + ); + + expect(failure).toMatchObject({ + code: 'COMMAND_FAILED', + details: { adbFailure: 'device_offline' }, + }); + expect(failure).not.toMatchObject({ details: { reason: 'android_port_reverse_rebind_refused' } }); +}); diff --git a/packages/platform-android/src/adb-port-reverse.ts b/packages/platform-android/src/adb-port-reverse.ts index 61f1802914..e6461cbb5c 100644 --- a/packages/platform-android/src/adb-port-reverse.ts +++ b/packages/platform-android/src/adb-port-reverse.ts @@ -6,22 +6,44 @@ import type { AndroidAdbProvider, AndroidPortReverseEndpoint, AndroidPortReverseMapping, + AndroidPortReverseOptions, AndroidPortReverseProvider, } from './adb-transport.ts'; // Port-reverse ownership: an owner-tracked manager over a provider's reverse capability (or an // exec-backed fallback), so concurrent sessions cannot silently steal each other's mappings. +export type AndroidExecPortReverseOptions = Readonly<{ + /** + * Refuses to replace a device mapping this provider did not create (`adb reverse --no-rebind`), + * for a device that other adb clients also drive. A refusal throws `COMMAND_FAILED` with + * `details.reason: 'android_port_reverse_rebind_refused'`. + */ + noRebind?: boolean; +}>; + +const ANDROID_PORT_REVERSE_REBIND_REFUSED_REASON = 'android_port_reverse_rebind_refused'; + const managedAndroidPortReverseProviders = new WeakSet(); export function createAndroidPortReverseManager( provider: AndroidAdbProvider | AndroidAdbExecutor, +): AndroidPortReverseProvider; +/** Options reach only the exec-backed provider the manager builds over a bare executor. */ +export function createAndroidPortReverseManager( + adb: AndroidAdbExecutor, + options: AndroidExecPortReverseOptions, +): AndroidPortReverseProvider; +export function createAndroidPortReverseManager( + provider: AndroidAdbProvider | AndroidAdbExecutor, + options?: AndroidExecPortReverseOptions, ): AndroidPortReverseProvider { const normalized = normalizeAndroidAdbProvider(provider); if (normalized.reverse && managedAndroidPortReverseProviders.has(normalized.reverse)) { return normalized.reverse; } - const reverse = normalized.reverse ?? createExecAndroidPortReverseProvider(normalized.exec); + const reverse = + normalized.reverse ?? createExecAndroidPortReverseProvider(normalized.exec, options); const active = new Map(); const manager: AndroidPortReverseProvider = { async ensure(mapping, options) { @@ -70,65 +92,90 @@ export function createAndroidPortReverseManager( export function createExecAndroidPortReverseProvider( adb: AndroidAdbExecutor, + providerOptions: AndroidExecPortReverseOptions = {}, ): AndroidPortReverseProvider { - const owned = new Map>(); + const bound = new Map(); + const list = async (options?: AndroidPortReverseOptions) => { + const result = await adb(['reverse', '--list'], { + allowFailure: true, + signal: options?.signal, + timeoutMs: options?.timeoutMs, + }); + if (result.exitCode !== 0) return []; + return parseAndroidReverseList(result.stdout, bound); + }; + const remove = async ( + local: AndroidPortReverseEndpoint, + options?: AndroidPortReverseOptions, + ): Promise => { + const result = await adb(['reverse', '--remove', local], { + allowFailure: true, + signal: options?.signal, + timeoutMs: options?.timeoutMs, + }); + if (result.exitCode !== 0 && !isMissingReverseMapping(result.stdout, result.stderr)) { + throw androidAdbResultError(`Failed to remove Android port reverse ${local}`, result, { + local, + }); + } + bound.delete(local); + }; return { async ensure(mapping, options) { - await adb(['reverse', mapping.local, mapping.remote], { - allowFailure: false, - signal: options?.signal, - timeoutMs: options?.timeoutMs, - }); - if (mapping.ownerId) { - const ownedLocals = owned.get(mapping.ownerId) ?? new Set(); - ownedLocals.add(mapping.local); - owned.set(mapping.ownerId, ownedLocals); - } - }, - async remove(local, options) { - const result = await adb(['reverse', '--remove', local], { - allowFailure: true, - signal: options?.signal, - timeoutMs: options?.timeoutMs, - }); - if (result.exitCode !== 0 && !isMissingReverseMapping(result.stdout, result.stderr)) { - throw androidAdbResultError(`Failed to remove Android port reverse ${local}`, result, { - local, - }); - } - for (const locals of owned.values()) { - locals.delete(local); + // A mapping this provider created stays rebindable; --no-rebind protects only foreign ones. + const noRebind = providerOptions.noRebind === true && !bound.has(mapping.local); + const result = await adb( + ['reverse', ...(noRebind ? ['--no-rebind'] : []), mapping.local, mapping.remote], + { allowFailure: noRebind, signal: options?.signal, timeoutMs: options?.timeoutMs }, + ); + if (result.exitCode !== 0) { + const existing = (await list(options)).find((listed) => listed.local === mapping.local); + throw existing + ? rebindRefusedError(mapping, existing) + : androidAdbResultError( + `Failed to ensure Android port reverse ${mapping.local}`, + result, + { + local: mapping.local, + remote: mapping.remote, + }, + ); } + bound.set(mapping.local, mapping.ownerId); }, + remove, async removeAllOwned(ownerId, options) { - const locals = [...(owned.get(ownerId) ?? [])]; + const locals = [...bound] + .filter(([, boundOwnerId]) => boundOwnerId === ownerId) + .map(([local]) => local); for (const local of locals) { - await this.remove(local, options); + await remove(local, options); } - owned.delete(ownerId); - }, - async list(options) { - const result = await adb(['reverse', '--list'], { - allowFailure: true, - signal: options?.signal, - timeoutMs: options?.timeoutMs, - }); - if (result.exitCode !== 0) return []; - return parseAndroidReverseList(result.stdout, owned); }, + list, }; } +function rebindRefusedError( + requested: AndroidPortReverseMapping, + existing: AndroidPortReverseMapping, +): AppError { + return new AppError( + 'COMMAND_FAILED', + `Android port reverse ${requested.local} is already mapped on the device by another adb client`, + { + reason: ANDROID_PORT_REVERSE_REBIND_REFUSED_REASON, + requested, + existing: { local: existing.local, remote: existing.remote }, + hint: `agent-device does not replace a mapping it did not create on a shared device. Free device ${requested.local} or use another device port.`, + }, + ); +} + function parseAndroidReverseList( stdout: string, - owned: ReadonlyMap>, + bound: ReadonlyMap, ): AndroidPortReverseMapping[] { - const ownerByLocal = new Map(); - for (const [ownerId, locals] of owned) { - for (const local of locals) { - ownerByLocal.set(local, ownerId); - } - } return stdout .split('\n') .map((line) => line.trim().split(/\s+/)) @@ -138,7 +185,7 @@ function parseAndroidReverseList( return { local: localEndpoint, remote: remote as AndroidPortReverseEndpoint, - ownerId: ownerByLocal.get(localEndpoint), + ownerId: bound.get(localEndpoint), }; }); } diff --git a/packages/platform-android/src/app-lifecycle.ts b/packages/platform-android/src/app-lifecycle.ts index c5880ce5fb..25ef98d4f6 100644 --- a/packages/platform-android/src/app-lifecycle.ts +++ b/packages/platform-android/src/app-lifecycle.ts @@ -132,6 +132,7 @@ async function ensureAndroidLocalhostReverse(device: DeviceInfo, target: string) }; if (error instanceof AppError) { Object.assign(details, { + reason: error.details?.reason, hint: error.details?.hint, diagnosticId: error.details?.diagnosticId, logPath: error.details?.logPath, diff --git a/website/docs/docs/client-api.md b/website/docs/docs/client-api.md index d59051db73..8376be31a8 100644 --- a/website/docs/docs/client-api.md +++ b/website/docs/docs/client-api.md @@ -83,7 +83,7 @@ Supported public entry points for Node consumers: - `agent-device/artifacts` - `resolveAndroidArchivePackageName(archivePath)` - `agent-device/android-adb` - - `createAndroidPortReverseManager(provider)` + - `createAndroidPortReverseManager(provider)` / `createAndroidPortReverseManager(executor, { noRebind })` - `captureAndroidLogcatWithAdb(executor, options?)` - `readAndroidClipboardWithAdb(executor)` / `writeAndroidClipboardWithAdb(executor, text)` - `getAndroidKeyboardStatusWithAdb(executor)` / `dismissAndroidKeyboardWithAdb(executor)` @@ -205,7 +205,10 @@ bounded logcat capture. Providers can also expose `reverse` for first-class port reverse ownership. Plain executors do not advertise reverse support automatically; call `createAndroidPortReverseManager(providerOrExecutor)` only when the provider supports `adb reverse` argument semantics. The manager makes duplicate setup -idempotent for the same owner and rejects conflicting owners for the same local endpoint. +idempotent for the same owner and rejects conflicting owners for the same local endpoint. For a +device that other adb clients also drive, pass an executor with `{ noRebind: true }`: the manager +runs `adb reverse --no-rebind` and does not replace a mapping it did not create. A refusal fails with +`COMMAND_FAILED` and `details.reason: 'android_port_reverse_rebind_refused'`. The device shell re-parses whatever follows `shell` or `exec-out`, so those commands are built for you: every dynamic word is rendered for the quoting its transport applies before it reaches the device. `adb` From 41bd8d3dba7e0cf39c95891c0667013004f338a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 08:20:03 +0200 Subject: [PATCH 2/5] fix(limrun): keep the owner's reverse mappings on attached Android instances createLimrunAndroidSession asks createPortReverse for no-rebind mode when ownership is 'attached'. configurePortReverse and the localhost URL auto-reverse can then no longer replace an owner mapping such as tcp:8081, so teardown cannot remove it. Created instances keep plain adb reverse. --- packages/provider-limrun/src/android.ts | 4 +- .../src/runtime-dependencies.ts | 9 +++- src/__tests__/limrun-runtime.test.ts | 42 +++++++++++++++++++ src/sdk/limrun-runtime-dependencies.ts | 4 +- website/docs/docs/limrun.md | 2 + 5 files changed, 57 insertions(+), 4 deletions(-) diff --git a/packages/provider-limrun/src/android.ts b/packages/provider-limrun/src/android.ts index 14c785e58d..9cb2e9c025 100644 --- a/packages/provider-limrun/src/android.ts +++ b/packages/provider-limrun/src/android.ts @@ -83,7 +83,9 @@ export async function createLimrunAndroidSession( await client.setText(request.target, request.text); }, }; - adbProvider.reverse = await dependencies.android.createPortReverse(adbProvider.exec); + adbProvider.reverse = await dependencies.android.createPortReverse(adbProvider.exec, { + noRebind: options.ownership === 'attached', + }); return Object.assign(session, { adbProvider }); } diff --git a/packages/provider-limrun/src/runtime-dependencies.ts b/packages/provider-limrun/src/runtime-dependencies.ts index d76bea717f..c22b9feabc 100644 --- a/packages/provider-limrun/src/runtime-dependencies.ts +++ b/packages/provider-limrun/src/runtime-dependencies.ts @@ -75,7 +75,14 @@ export type LimrunAndroidKeyboardDismissResult = LimrunAndroidKeyboardState & { export type LimrunAndroidRuntimeAdapter = { // Interactors need provider-scoped capabilities; command helpers below need only ADB execution. createInteractor(device: DeviceInfo, adb: LimrunAdbProvider): Interactor; - createPortReverse(adb: LimrunAdbExecutor): Promise; + /** + * `noRebind` refuses to replace a device mapping this session did not create, for an instance + * whose owner may hold reverse mappings of their own. + */ + createPortReverse( + adb: LimrunAdbExecutor, + options: Readonly<{ noRebind: boolean }>, + ): Promise; inferAppName(packageName: string): Promise; listApps( adb: LimrunAdbExecutor, diff --git a/src/__tests__/limrun-runtime.test.ts b/src/__tests__/limrun-runtime.test.ts index e06499d417..1136b1a84c 100644 --- a/src/__tests__/limrun-runtime.test.ts +++ b/src/__tests__/limrun-runtime.test.ts @@ -864,6 +864,48 @@ test('Limrun removes only its own port reverse mappings from an attached Android assert.deepEqual(removals, [['-s', '127.0.0.1:62001', 'reverse', '--remove', 'tcp:8097']]); }); +test('Limrun refuses to replace an owner port reverse on an attached Android instance', async () => { + const runtime = new LimrunRuntime({ instances: { android: ATTACHED_ANDROID } }); + const lease = { ...androidLease(), leaseId: 'lease-attached-android' }; + vi.mocked(runCmd).mockImplementation(async (_command, args) => + args.includes('--no-rebind') + ? { stdout: '', stderr: 'adb: error: cannot rebind existing socket', exitCode: 1 } + : { + stdout: args.includes('--list') ? 'owner-host tcp:8081 tcp:8081\n' : '', + stderr: '', + exitCode: 0, + }, + ); + const isRebindRefusal = (error: unknown) => + (error as { details?: { reason?: unknown } }).details?.reason === + 'android_port_reverse_rebind_refused'; + + const device = await allocateLimrunDevice(runtime, lease); + await assert.rejects( + () => + runtime.configurePortReverse({ + leaseId: lease.leaseId, + devicePort: 8081, + hostPort: 8081, + name: 'metro', + }), + isRebindRefusal, + ); + const interactor = runtime.getInteractor(device); + if (!interactor) throw new Error('Limrun runtime must return an interactor'); + await assert.rejects(() => interactor.open('exp://127.0.0.1:8081'), isRebindRefusal); + await runtime.shutdown(); + + const reverseCalls = vi + .mocked(runCmd) + .mock.calls.map(([, args]) => args.slice(2)) + .filter((args) => args[0] === 'reverse' && args[1] !== '--list'); + assert.deepEqual(reverseCalls, [ + ['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081'], + ['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081'], + ]); +}); + test('Limrun instance access wins over the API key for its platform only', async () => { const runtime = new LimrunRuntime({ apiKey: 'lim_test_key', instances: { ios: ATTACHED_IOS } }); diff --git a/src/sdk/limrun-runtime-dependencies.ts b/src/sdk/limrun-runtime-dependencies.ts index 87fa46dc08..222681e32f 100644 --- a/src/sdk/limrun-runtime-dependencies.ts +++ b/src/sdk/limrun-runtime-dependencies.ts @@ -18,10 +18,10 @@ export function createLimrunRuntimeDependencies(): LimrunRuntimeDependencies { clientVersion: readVersion(), android: { createInteractor: (device, adb) => createAndroidInteractor(device, adb), - createPortReverse: async (adb) => { + createPortReverse: async (adb, options) => { const { createAndroidPortReverseManager } = await import('@agent-device/platform-android/mechanics'); - return createAndroidPortReverseManager(adb); + return createAndroidPortReverseManager(adb, options); }, inferAppName: async (packageName) => { const { inferAndroidAppName } = await import('@agent-device/platform-android/mechanics'); diff --git a/website/docs/docs/limrun.md b/website/docs/docs/limrun.md index 7e23faf838..850b081914 100644 --- a/website/docs/docs/limrun.md +++ b/website/docs/docs/limrun.md @@ -41,6 +41,8 @@ agent-device disconnect `connect` checks the instance credentials. agent-device never creates or deletes that instance: `disconnect` leaves it running, and its owner deletes it. When the variables for a platform are set, they take precedence over `LIMRUN_API_KEY` for that platform. +On Android, agent-device does not replace a port reverse mapping it did not create. If the owner already maps a device port, such as `tcp:8081` for their Metro server, a reverse to that port fails with `details.reason: 'android_port_reverse_rebind_refused'`, and the owner's mapping stays in place. + `install`, and `apps` before the first `open`, still need `LIMRUN_API_KEY`, because they use Limrun asset storage. After `open`, `apps` lists the apps installed on the instance without the key. Install the app before you hand over the instance. From the Node.js runtime, `getDeviceSession(device).installRemoteApp(url)` installs from a signed asset URL without the API key. ## Keeping idle sessions alive From 588ecc61275ef21c438fa8718905ba69515ad5e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 08:37:16 +0200 Subject: [PATCH 3/5] fix(android): never rebind an existing mapping in no-rebind mode No-rebind mode now passes --no-rebind on every ensure, so a stale record of a mapping this provider bound can no longer turn into a plain adb reverse over another client's mapping. The manager serializes ensures per device endpoint, so a concurrent duplicate sees the first mapping instead of a refusal, and two owners can no longer both pass its ownership check. Docs state when the typed reason is available. --- .../src/adb-port-reverse.test.ts | 51 +++++++++++++++++-- .../platform-android/src/adb-port-reverse.ts | 44 ++++++++-------- website/docs/docs/client-api.md | 5 +- website/docs/docs/limrun.md | 2 +- 4 files changed, 75 insertions(+), 27 deletions(-) diff --git a/packages/platform-android/src/adb-port-reverse.test.ts b/packages/platform-android/src/adb-port-reverse.test.ts index 66f4b51244..099c6e668f 100644 --- a/packages/platform-android/src/adb-port-reverse.test.ts +++ b/packages/platform-android/src/adb-port-reverse.test.ts @@ -99,26 +99,69 @@ test('no-rebind refuses a device mapping another client owns with a typed reason ]); }); -test('no-rebind still rebinds a mapping the same provider created', async () => { +test('no-rebind does not re-point a mapping the same provider created', async () => { bindAndroidAdbHostStub(); const calls: (readonly string[])[] = []; + const device = new Map(); const manager = createAndroidPortReverseManager( async (args) => { calls.push(args); + if (args[1] === '--list') { + return ok([...device].map(([local, remote]) => `host-1 ${local} ${remote}\n`).join('')); + } + const [local, remote] = args.slice(-2) as [string, string]; + if (args.includes('--no-rebind') && device.has(local)) { + return { exitCode: 1, stdout: '', stderr: 'adb: error: cannot rebind existing socket' }; + } + device.set(local, remote); return ok(); }, { noRebind: true }, ); await manager.ensure({ local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' }); - await manager.ensure({ local: 'tcp:8081', remote: 'tcp:9090', ownerId: 'metro' }); + await expect( + manager.ensure({ local: 'tcp:8081', remote: 'tcp:9090', ownerId: 'metro' }), + ).rejects.toMatchObject({ + details: { + reason: 'android_port_reverse_rebind_refused', + existing: { local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' }, + }, + }); - expect(calls).toEqual([ + expect(device.get('tcp:8081')).toBe('tcp:8081'); + expect(calls.filter((args) => args[1] !== '--list')).toEqual([ ['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081'], - ['reverse', 'tcp:8081', 'tcp:9090'], + ['reverse', '--no-rebind', 'tcp:8081', 'tcp:9090'], ]); }); +test('concurrent ensures of one endpoint reach the device once', async () => { + bindAndroidAdbHostStub(); + const binds: (readonly string[])[] = []; + const device = new Set(); + const manager = createAndroidPortReverseManager( + async (args) => { + if (args[1] === '--list') + return ok([...device].map((local) => `host-1 ${local} ${local}\n`).join('')); + binds.push(args); + const local = args.at(-2) as string; + if (device.has(local)) { + return { exitCode: 1, stdout: '', stderr: 'adb: error: cannot rebind existing socket' }; + } + device.add(local); + await new Promise((resolve) => setTimeout(resolve, 5)); + return ok(); + }, + { noRebind: true }, + ); + const mapping = { local: 'tcp:8081', remote: 'tcp:8081', ownerId: 'metro' } as const; + + await Promise.all([manager.ensure(mapping), manager.ensure(mapping)]); + + expect(binds).toEqual([['reverse', '--no-rebind', 'tcp:8081', 'tcp:8081']]); +}); + test('no-rebind reports an adb failure when the device lists no mapping for the endpoint', async () => { bindAndroidAdbHostStub(); const manager = createAndroidPortReverseManager( diff --git a/packages/platform-android/src/adb-port-reverse.ts b/packages/platform-android/src/adb-port-reverse.ts index e6461cbb5c..19bc07a458 100644 --- a/packages/platform-android/src/adb-port-reverse.ts +++ b/packages/platform-android/src/adb-port-reverse.ts @@ -1,4 +1,5 @@ import { AppError } from '@agent-device/kernel/errors'; +import { withKeyedLock } from '@agent-device/kernel/keyed-lock'; import { androidAdbResultError } from './adb-failure.ts'; import { normalizeAndroidAdbProvider } from './adb-provider-normalization.ts'; import type { @@ -15,8 +16,9 @@ import type { export type AndroidExecPortReverseOptions = Readonly<{ /** - * Refuses to replace a device mapping this provider did not create (`adb reverse --no-rebind`), - * for a device that other adb clients also drive. A refusal throws `COMMAND_FAILED` with + * Refuses to replace any existing device mapping, including one this provider created + * (`adb reverse --no-rebind`), for a device that other adb clients also drive. A refusal the + * device listing confirms throws `COMMAND_FAILED` with * `details.reason: 'android_port_reverse_rebind_refused'`. */ noRebind?: boolean; @@ -45,21 +47,24 @@ export function createAndroidPortReverseManager( const reverse = normalized.reverse ?? createExecAndroidPortReverseProvider(normalized.exec, options); const active = new Map(); + const ensuring = new Map>(); const manager: AndroidPortReverseProvider = { async ensure(mapping, options) { - const current = active.get(mapping.local); - if (current && current.ownerId !== mapping.ownerId) { - throw new AppError( - 'COMMAND_FAILED', - `Android port reverse ${mapping.local} is already owned by ${current.ownerId ?? 'another session'}`, - { current, requested: mapping }, - ); - } - if (current?.remote === mapping.remote) { - return; - } - await reverse.ensure(mapping, options); - active.set(mapping.local, { ...mapping }); + await withKeyedLock(ensuring, mapping.local, async () => { + const current = active.get(mapping.local); + if (current && current.ownerId !== mapping.ownerId) { + throw new AppError( + 'COMMAND_FAILED', + `Android port reverse ${mapping.local} is already owned by ${current.ownerId ?? 'another session'}`, + { current, requested: mapping }, + ); + } + if (current?.remote === mapping.remote) { + return; + } + await reverse.ensure(mapping, options); + active.set(mapping.local, { ...mapping }); + }); }, async remove(local, options) { if (!active.has(local)) { @@ -122,8 +127,7 @@ export function createExecAndroidPortReverseProvider( }; return { async ensure(mapping, options) { - // A mapping this provider created stays rebindable; --no-rebind protects only foreign ones. - const noRebind = providerOptions.noRebind === true && !bound.has(mapping.local); + const noRebind = providerOptions.noRebind === true; const result = await adb( ['reverse', ...(noRebind ? ['--no-rebind'] : []), mapping.local, mapping.remote], { allowFailure: noRebind, signal: options?.signal, timeoutMs: options?.timeoutMs }, @@ -162,12 +166,12 @@ function rebindRefusedError( ): AppError { return new AppError( 'COMMAND_FAILED', - `Android port reverse ${requested.local} is already mapped on the device by another adb client`, + `Android port reverse ${requested.local} is already mapped on the device`, { reason: ANDROID_PORT_REVERSE_REBIND_REFUSED_REASON, requested, - existing: { local: existing.local, remote: existing.remote }, - hint: `agent-device does not replace a mapping it did not create on a shared device. Free device ${requested.local} or use another device port.`, + existing, + hint: `agent-device does not replace an existing reverse mapping on a shared device. Remove the mapping that holds device ${requested.local}, or use another device port.`, }, ); } diff --git a/website/docs/docs/client-api.md b/website/docs/docs/client-api.md index 8376be31a8..e66c31c251 100644 --- a/website/docs/docs/client-api.md +++ b/website/docs/docs/client-api.md @@ -207,8 +207,9 @@ advertise reverse support automatically; call `createAndroidPortReverseManager(p only when the provider supports `adb reverse` argument semantics. The manager makes duplicate setup idempotent for the same owner and rejects conflicting owners for the same local endpoint. For a device that other adb clients also drive, pass an executor with `{ noRebind: true }`: the manager -runs `adb reverse --no-rebind` and does not replace a mapping it did not create. A refusal fails with -`COMMAND_FAILED` and `details.reason: 'android_port_reverse_rebind_refused'`. +runs `adb reverse --no-rebind` and never replaces an existing device mapping, including one it +created. When `adb reverse --list` shows the mapping, the refusal fails with `COMMAND_FAILED` and +`details.reason: 'android_port_reverse_rebind_refused'`. Otherwise it fails as an ordinary adb error. The device shell re-parses whatever follows `shell` or `exec-out`, so those commands are built for you: every dynamic word is rendered for the quoting its transport applies before it reaches the device. `adb` diff --git a/website/docs/docs/limrun.md b/website/docs/docs/limrun.md index 850b081914..354936caf9 100644 --- a/website/docs/docs/limrun.md +++ b/website/docs/docs/limrun.md @@ -41,7 +41,7 @@ agent-device disconnect `connect` checks the instance credentials. agent-device never creates or deletes that instance: `disconnect` leaves it running, and its owner deletes it. When the variables for a platform are set, they take precedence over `LIMRUN_API_KEY` for that platform. -On Android, agent-device does not replace a port reverse mapping it did not create. If the owner already maps a device port, such as `tcp:8081` for their Metro server, a reverse to that port fails with `details.reason: 'android_port_reverse_rebind_refused'`, and the owner's mapping stays in place. +On Android, agent-device does not replace an existing port reverse mapping. If the owner already maps a device port, such as `tcp:8081` for their Metro server, a reverse to that port fails and the owner's mapping stays in place. The error has `details.reason: 'android_port_reverse_rebind_refused'` when `adb reverse --list` shows the mapping. Otherwise it is a plain ADB failure. `install`, and `apps` before the first `open`, still need `LIMRUN_API_KEY`, because they use Limrun asset storage. After `open`, `apps` lists the apps installed on the instance without the key. Install the app before you hand over the instance. From the Node.js runtime, `getDeviceSession(device).installRemoteApp(url)` installs from a signed asset URL without the API key. From e4bba1ff7e0bbf97fe23bbfae502fe9abeb66669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 08:48:48 +0200 Subject: [PATCH 4/5] docs(android): state when the no-rebind refusal carries its typed reason Name adb reverse --list as the condition in the noRebind JSDoc, and scope the Limrun docs guarantee to attached Android instances. --- packages/platform-android/src/adb-port-reverse.ts | 7 ++++--- website/docs/docs/limrun.md | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/platform-android/src/adb-port-reverse.ts b/packages/platform-android/src/adb-port-reverse.ts index 19bc07a458..23b9e26ef4 100644 --- a/packages/platform-android/src/adb-port-reverse.ts +++ b/packages/platform-android/src/adb-port-reverse.ts @@ -17,9 +17,10 @@ import type { export type AndroidExecPortReverseOptions = Readonly<{ /** * Refuses to replace any existing device mapping, including one this provider created - * (`adb reverse --no-rebind`), for a device that other adb clients also drive. A refusal the - * device listing confirms throws `COMMAND_FAILED` with - * `details.reason: 'android_port_reverse_rebind_refused'`. + * (`adb reverse --no-rebind`), for a device that other adb clients also drive. When + * `adb reverse --list` shows the endpoint after a refusal, the provider throws `COMMAND_FAILED` + * with `details.reason: 'android_port_reverse_rebind_refused'`; otherwise it throws the adb + * failure. */ noRebind?: boolean; }>; diff --git a/website/docs/docs/limrun.md b/website/docs/docs/limrun.md index 354936caf9..ea5134fa2a 100644 --- a/website/docs/docs/limrun.md +++ b/website/docs/docs/limrun.md @@ -41,7 +41,7 @@ agent-device disconnect `connect` checks the instance credentials. agent-device never creates or deletes that instance: `disconnect` leaves it running, and its owner deletes it. When the variables for a platform are set, they take precedence over `LIMRUN_API_KEY` for that platform. -On Android, agent-device does not replace an existing port reverse mapping. If the owner already maps a device port, such as `tcp:8081` for their Metro server, a reverse to that port fails and the owner's mapping stays in place. The error has `details.reason: 'android_port_reverse_rebind_refused'` when `adb reverse --list` shows the mapping. Otherwise it is a plain ADB failure. +On an attached Android instance, agent-device does not replace an existing port reverse mapping. If the owner already maps a device port, such as `tcp:8081` for their Metro server, a reverse to that port fails and the owner's mapping stays in place. The error has `details.reason: 'android_port_reverse_rebind_refused'` when `adb reverse --list` shows the mapping. Otherwise it is a plain ADB failure. `install`, and `apps` before the first `open`, still need `LIMRUN_API_KEY`, because they use Limrun asset storage. After `open`, `apps` lists the apps installed on the instance without the key. Install the app before you hand over the instance. From the Node.js runtime, `getDeviceSession(device).installRemoteApp(url)` installs from a signed asset URL without the API key. From 356932729c7ba441928957d76bad04b80440ed71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 4 Oct 2026 21:08:16 +0200 Subject: [PATCH 5/5] fix(android): give localhost URL reverses an owner The localhost URL auto-reverse now ensures its mapping with the stable owner 'localhost-url'. Teardown of an attached Limrun instance removes only owned mappings, so before this change the mapping stayed on the owner's device after disconnect. A live run showed that adbd keeps it. --- .../src/__tests__/app-lifecycle-open.test.ts | 12 +++++++++--- packages/platform-android/src/app-lifecycle.ts | 8 +++++++- src/__tests__/limrun-runtime.test.ts | 13 +++++++++---- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/packages/platform-android/src/__tests__/app-lifecycle-open.test.ts b/packages/platform-android/src/__tests__/app-lifecycle-open.test.ts index d4734d204a..5e2fae25c3 100644 --- a/packages/platform-android/src/__tests__/app-lifecycle-open.test.ts +++ b/packages/platform-android/src/__tests__/app-lifecycle-open.test.ts @@ -171,7 +171,8 @@ test('openAndroidApp ensures Android reverse before localhost deep link launch', booted: true, }; const calls: Array< - { kind: 'exec'; args: readonly string[] } | { kind: 'reverse'; local: string; remote: string } + | { kind: 'exec'; args: readonly string[] } + | { kind: 'reverse'; local: string; remote: string; ownerId?: string } > = []; await withAndroidAdbProvider( @@ -182,7 +183,12 @@ test('openAndroidApp ensures Android reverse before localhost deep link launch', }, reverse: { ensure: async (mapping) => { - calls.push({ kind: 'reverse', local: mapping.local, remote: mapping.remote }); + calls.push({ + kind: 'reverse', + local: mapping.local, + remote: mapping.remote, + ownerId: mapping.ownerId, + }); }, remove: async () => {}, removeAllOwned: async () => {}, @@ -193,7 +199,7 @@ test('openAndroidApp ensures Android reverse before localhost deep link launch', ); assert.deepEqual(calls, [ - { kind: 'reverse', local: 'tcp:8083', remote: 'tcp:8083' }, + { kind: 'reverse', local: 'tcp:8083', remote: 'tcp:8083', ownerId: 'localhost-url' }, { kind: 'exec', args: [ diff --git a/packages/platform-android/src/app-lifecycle.ts b/packages/platform-android/src/app-lifecycle.ts index 25ef98d4f6..8019768135 100644 --- a/packages/platform-android/src/app-lifecycle.ts +++ b/packages/platform-android/src/app-lifecycle.ts @@ -28,6 +28,8 @@ const ANDROID_LAUNCHER_CATEGORY = 'android.intent.category.LAUNCHER'; const ANDROID_LEANBACK_CATEGORY = 'android.intent.category.LEANBACK_LAUNCHER'; const ANDROID_DEFAULT_CATEGORY = 'android.intent.category.DEFAULT'; const ANDROID_LOCALHOST_HOSTNAMES = new Set(['localhost', '127.0.0.1', '::1', '[::1]']); +/** Owns localhost URL reverses, so a teardown that removes only owned mappings removes them too. */ +const ANDROID_LOCALHOST_URL_REVERSE_OWNER = 'localhost-url'; const ANDROID_CLOSE_FOCUS_TIMEOUT_MS = 2_000; const ANDROID_CLOSE_FOCUS_POLL_MS = 50; const ANDROID_CLOSE_PROCESS_TIMEOUT_MS = 2_000; @@ -124,7 +126,11 @@ async function ensureAndroidLocalhostReverse(device: DeviceInfo, target: string) const reverse = createAndroidPortReverseManager(resolveAndroidAdbProvider(device)); try { - await reverse.ensure({ local: endpoint, remote: endpoint }); + await reverse.ensure({ + local: endpoint, + remote: endpoint, + ownerId: ANDROID_LOCALHOST_URL_REVERSE_OWNER, + }); } catch (error) { const details = { localPort: endpoint.replace('tcp:', ''), diff --git a/src/__tests__/limrun-runtime.test.ts b/src/__tests__/limrun-runtime.test.ts index 1136b1a84c..2d4141d1f8 100644 --- a/src/__tests__/limrun-runtime.test.ts +++ b/src/__tests__/limrun-runtime.test.ts @@ -843,25 +843,30 @@ test('Limrun removes only its own port reverse mappings from an attached Android const runtime = new LimrunRuntime({ instances: { android: ATTACHED_ANDROID } }); const lease = { ...androidLease(), leaseId: 'lease-attached-android' }; vi.mocked(runCmd).mockImplementation(async (_command, args) => ({ - stdout: args.includes('--list') ? 'host-7 tcp:8081 tcp:8081\nhost-9 tcp:8097 tcp:8097\n' : '', + stdout: args.includes('--list') + ? 'host-7 tcp:8081 tcp:8081\nhost-9 tcp:8097 tcp:8097\nhost-9 tcp:8099 tcp:8099\n' + : '', stderr: '', exitCode: 0, })); - await allocateLimrunDevice(runtime, lease); + const device = await allocateLimrunDevice(runtime, lease); await runtime.configurePortReverse({ leaseId: lease.leaseId, devicePort: 8097, hostPort: 8097, name: 'react-devtools', }); + await runtime.getInteractor(device)?.open('http://127.0.0.1:8099/'); await runtime.shutdown(); const removals = vi .mocked(runCmd) .mock.calls.map(([, args]) => args) - .filter((args) => args.includes('--remove')); - assert.deepEqual(removals, [['-s', '127.0.0.1:62001', 'reverse', '--remove', 'tcp:8097']]); + .filter((args) => args.includes('--remove')) + .map((args) => args.at(-1)) + .sort(); + assert.deepEqual(removals, ['tcp:8097', 'tcp:8099']); }); test('Limrun refuses to replace an owner port reverse on an attached Android instance', async () => {