diff --git a/packages/kernel/src/contracts.ts b/packages/kernel/src/contracts.ts index 1ef172702a..9c18499d09 100644 --- a/packages/kernel/src/contracts.ts +++ b/packages/kernel/src/contracts.ts @@ -2,7 +2,7 @@ import type { DaemonError } from './errors.ts'; export type { AppErrorCode } from './errors.ts'; export { defaultHintForCode, normalizeError } from './errors.ts'; -import type { PlatformSelector } from './device.ts'; +import type { PlatformSelector, PublicPlatform } from './device.ts'; const SESSION_RUNTIME_PLATFORMS = ['ios', 'android', 'harmonyos'] as const; export type SessionRuntimePlatform = (typeof SESSION_RUNTIME_PLATFORMS)[number]; @@ -58,6 +58,54 @@ const LEASE_BACKENDS = [ 'harmonyos-instance', ] as const; export type LeaseBackend = (typeof LEASE_BACKENDS)[number]; + +// Which lease backend rents a device on each platform the remote lease layer can hold. `ios-simulator` +// is a backend-specific runner guard rather than something a platform selector names, and the +// platforms with no remote lease backend (`vega`, `linux`, `web`) — and the macOS desktop host — map +// to no backend at all, so a request for one fails on the missing backend instead of renting a +// device no provider owns. Keyed on the `--platform` selector axis: callers holding a `DeviceInfo` +// project it with `publicPlatformString` first, which is the axis #2962 mixed up. +const LEASE_BACKEND_BY_PLATFORM: Partial> = { + ios: 'ios-instance', + android: 'android-instance', + harmonyos: 'harmonyos-instance', +}; + +/** + * Maps a platform to the lease backend that rents it. The CLI reads it for `--platform`/ + * `--lease-backend` resolution and the remote connection reads it for the device it just resolved. + * Both previously keyed their own copy off a platform axis, which is where #2962 started; a further + * copy in `connect limrun` validation is tracked for follow-up. + */ +export function leaseBackendForPlatform( + platform: PlatformSelector | undefined, +): LeaseBackend | undefined { + return platform === undefined ? undefined : LEASE_BACKEND_BY_PLATFORM[platform]; +} + +/** + * The public leaf platform a lease backend rents devices on — the inverse of + * {@link leaseBackendForPlatform} for the backends that name a platform rather than a runner guard. + * + * A connection binds a platform at the same moment it binds a lease, and the lease is the stronger + * evidence: it names the backend that is actually holding the device. `ios-simulator` maps to no + * leaf because it is a runner/process guard below device leases, not a platform a selector names. + * + * Derived from the forward table rather than written beside it, so the two axes cannot drift, and held + * in a `Map` because a `leaseBackend` reaching here can be any string an older binary left on disk: a + * plain object would answer `constructor` and friends with an inherited function, which is a platform + * nobody rents. + */ +const PLATFORM_BY_LEASE_BACKEND = new Map( + Object.entries(LEASE_BACKEND_BY_PLATFORM).flatMap(([platform, backend]) => + backend === undefined ? [] : [[backend, platform as PublicPlatform] as const], + ), +); + +export function platformForLeaseBackend(backend: string): PublicPlatform | undefined { + return PLATFORM_BY_LEASE_BACKEND.get(backend); +} + const DAEMON_SERVER_MODES = ['socket', 'http', 'dual'] as const; export type DaemonServerMode = (typeof DAEMON_SERVER_MODES)[number]; const DAEMON_TRANSPORT_PREFERENCES = ['auto', 'socket', 'http'] as const; diff --git a/packages/kernel/src/device.ts b/packages/kernel/src/device.ts index 4aeaa40641..5f435f138d 100644 --- a/packages/kernel/src/device.ts +++ b/packages/kernel/src/device.ts @@ -249,6 +249,31 @@ export function matchesPlatformSelector( return device.platform === selector; } +/** + * Whether two `--platform` selections can name the SAME device. + * + * Selectors name a platform on one of two axes: the collapsed `apple` family, or an Apple leaf + * (`ios`/`macos`) — plus the non-Apple platforms, which have one axis each. Equality of the two + * strings is therefore not the question: `apple` and `ios` name overlapping devices while `ios` and + * `macos` do not. The `apple` selector is only equivalent to a leaf, never to a non-Apple platform. + * + * Comparing selectors by string instead was the shape behind #2962, where a remote connection bound + * to the public `ios` was compared with an `apple`-axis value and every iOS install was refused. + * Any caller that decides "this request targets a different platform than the one already bound" + * has to answer it on both axes, which is why this lives beside the selectors rather than in one + * caller. + */ +export function platformSelectorsConflict( + requested: PlatformSelector | undefined, + bound: PlatformSelector | undefined, +): boolean { + if (!requested || !bound) return false; + if (requested === bound) return false; + if (requested === 'apple') return !isApplePlatform(bound); + if (bound === 'apple') return !isApplePlatform(requested); + return true; +} + export function resolveApplePlatformName( platformOrTarget: ApplePlatform | DeviceTarget | undefined, appleOs?: AppleOS, @@ -415,14 +440,27 @@ function deviceIdentityMistakenForNameHint( if (!flag) return undefined; return ( `${deviceName} is the id of ${JSON.stringify(identityMatch.name)}, not its name. ` + - `Did you mean ${flag} ${deviceName}?` + `Did you mean --${flag} ${deviceName}?` ); } -/** The identity flag that can actually resolve a device on this platform, if one exists. */ -function deviceIdentityFlag(platform: Platform): '--udid' | '--serial' | undefined { - if (isApplePlatform(platform)) return '--udid'; - if (isSerialAddressablePlatform(platform)) return '--serial'; +export type DeviceIdentityFlag = 'udid' | 'serial'; + +/** + * Which flag carries a device identity on a platform: `udid` addresses Apple devices, `serial` + * addresses the serial-addressable ones. Resolution rejects the wrong pairing + * (`assertSelectorFlagMatchesPlatform`), and a caller that resolved a device and has to re-issue it + * as flags — a remote lease request that must bind the device it just picked — has to name the same + * flag, or the two drift and the request binds a selector that resolves a DIFFERENT device. + * + * Two sites still spell the pairing out inline; each differs from this rule in a way that is its own + * decision, so they are tracked as follow-ups rather than folded in here. + */ +export function deviceIdentityFlag( + platform: Platform | PublicPlatform, +): DeviceIdentityFlag | undefined { + if (isApplePlatform(platform)) return 'udid'; + if (isSerialAddressablePlatform(platform)) return 'serial'; return undefined; } @@ -472,10 +510,9 @@ function throwAmbiguousDeviceSelection(candidates: DeviceInfo[]): never { function buildAmbiguousDeviceHint(candidates: DeviceInfo[]): string { const first = candidates[0]; - const identitySelector = - first && isSerialAddressablePlatform(first.platform) - ? `--serial ${first.id}` - : `--udid ${first?.id ?? ''}`; + const identitySelector = first + ? `--${deviceIdentityFlag(first.platform) ?? 'udid'} ${first.id}` + : `--udid `; return ( `Select the intended device explicitly, for example ${identitySelector} ` + `or --device ${JSON.stringify(first?.name ?? '')}. ` + diff --git a/src/__tests__/remote-connection-harmonyos.test.ts b/src/__tests__/remote-connection-harmonyos.test.ts index f35b4a0923..3ecfc8e8bb 100644 --- a/src/__tests__/remote-connection-harmonyos.test.ts +++ b/src/__tests__/remote-connection-harmonyos.test.ts @@ -26,10 +26,77 @@ test('HarmonyOS platform resolves to its proxy lease backend', () => { ).toBe('harmonyos-instance'); }); -test.each(['apple', 'harmonyos', 'ios', 'android'] as const)( - 'stored Harmony runtime compatibility respects %s selection', +test('stored Harmony runtime compatibility keeps the HarmonyOS runtime for a HarmonyOS selection', async () => { + const { stateDir, remoteConfigPath } = connectionWorkspace('harmonyos-runtime-'); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'runtime-compat', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'proxy', + runId: 'compat-run', + leaseId: 'compat-existing', + leaseBackend: 'harmonyos-instance', + runtime: { platform: 'harmonyos', launchUrl: 'demo://open' }, + }, + }); + const materialized = await materializeRemoteConnectionForCommand({ + command: 'snapshot', + client: createTestClient(), + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + session: 'runtime-compat', + platform: 'harmonyos', + }, + }); + expect(materialized.runtime?.platform).toBe('harmonyos'); +}); + +/** + * A backend that rents HarmonyOS devices is the device this connection holds, even when the record + * never wrote a platform, so any other family is refused rather than sent along the existing lease + * (#2962). Before the backend was read, `apple` silently ran as a HarmonyOS snapshot and `ios` went + * out as an `ios` snapshot — both against a `harmonyos-instance` lease. + */ +async function expectRejectedSelection( + stateDir: string, + remoteConfigPath: string, + platform: 'apple' | 'ios' | 'android', +): Promise { + await expect( + materializeRemoteConnectionForCommand({ + command: 'snapshot', + client: createTestClient(), + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + session: 'runtime-compat', + platform, + }, + }), + ).rejects.toMatchObject({ + code: 'INVALID_ARGS', + details: { + reason: 'CONNECTION_PLATFORM_CONFLICT', + platform: 'harmonyos', + requestedPlatform: platform, + }, + }); +} + +test.each(['apple', 'ios', 'android'] as const)( + 'a harmonyos-instance lease refuses a %s selection', async (platform) => { - const { stateDir, remoteConfigPath } = connectionWorkspace('harmonyos-runtime-'); + const { stateDir, remoteConfigPath } = connectionWorkspace('harmonyos-runtime-reject-'); fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); seedConnectionState({ stateDir, @@ -44,24 +111,7 @@ test.each(['apple', 'harmonyos', 'ios', 'android'] as const)( runtime: { platform: 'harmonyos', launchUrl: 'demo://open' }, }, }); - const materialized = await materializeRemoteConnectionForCommand({ - command: 'snapshot', - client: createTestClient(), - flags: { - json: true, - help: false, - version: false, - stateDir, - remoteConfig: remoteConfigPath, - session: 'runtime-compat', - platform, - }, - }); - if (platform === 'apple' || platform === 'harmonyos') { - expect(materialized.runtime?.platform).toBe('harmonyos'); - } else { - expect(materialized.runtime).toBeUndefined(); - } + await expectRejectedSelection(stateDir, remoteConfigPath, platform); }, ); diff --git a/src/__tests__/remote-connection-platform-axis.test.ts b/src/__tests__/remote-connection-platform-axis.test.ts new file mode 100644 index 0000000000..02b16927be --- /dev/null +++ b/src/__tests__/remote-connection-platform-axis.test.ts @@ -0,0 +1,790 @@ +// The platform axis a remote connection is named on (#2962). +// +// A `DeviceInfo` carries the INTERNAL `apple` platform while everything a connection records or +// sends — `--platform`, the connection state, the proxy device key, the lease request — speaks the +// PUBLIC leaf (`ios`/`macos`, ADR 0009). Comparing the two axes by string equality refused every +// iOS install and open on a proxy lease and demanded `connect --force` for a connection that had +// not changed. These pin both directions: the family and leaf selectors name the same devices, and +// a genuinely different platform is still refused. +// +// Kept out of `remote-connection.test.ts`, which is already over the test-file size tripwire and +// may not grow (docs/agents/testing.md). + +import { afterEach, test, vi } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { + connectionWorkspace, + createTestClient, + recordedLeaseAllocate, + seedConnectionState, +} from './remote-connection.fixtures.ts'; +import { connectCommand } from '../cli/commands/connection.ts'; +import { materializeRemoteConnectionForCommand } from '../cli/commands/connection-runtime.ts'; +import { AppError } from '@agent-device/kernel/errors'; +import { readRemoteConnectionState } from '../remote/remote-connection-state.ts'; + +afterEach(() => { + vi.clearAllMocks(); + vi.restoreAllMocks(); + vi.unstubAllEnvs(); +}); + +test('proxy install against an iOS-bound connection is not refused as a platform change', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-proxy-ios-bound-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-proxy', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'proxy', + runId: 'proxy-client-1', + leaseProvider: 'proxy', + clientId: 'client-1', + platform: 'ios', + leaseBackend: 'ios-instance', + }, + }); + const allocate = recordedLeaseAllocate({ leaseId: 'ios-lease-1', backend: 'ios-instance' }); + + const materialized = await materializeRemoteConnectionForCommand({ + command: 'install', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'proxy', + runId: 'proxy-client-1', + session: 'adc-proxy', + platform: 'ios', + }, + client: createTestClient({ + listDevices: async () => [ + { + platform: 'ios', + target: 'mobile', + kind: 'simulator', + id: 'SIM-001', + name: 'iPhone 16', + booted: true, + identifiers: { udid: 'SIM-001' }, + ios: { udid: 'SIM-001' }, + }, + ], + allocate: allocate.stub, + }), + }); + + assert.equal(materialized.flags.leaseId, 'ios-lease-1'); + assert.equal(allocate.request?.deviceKey, 'ios:mobile:SIM-001'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// The connection records a leaf; `--platform apple` names the same devices. Comparing the two by +// string equality refused the request and demanded `connect --force` for a connection that had not +// changed at all (#2962). +test('proxy install with the apple family selector matches an ios-bound connection', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-proxy-apple-selector-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-proxy', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'proxy', + runId: 'proxy-client-1', + leaseProvider: 'proxy', + clientId: 'client-1', + platform: 'ios', + leaseBackend: 'ios-instance', + }, + }); + const allocate = recordedLeaseAllocate({ leaseId: 'ios-lease-1', backend: 'ios-instance' }); + + await materializeRemoteConnectionForCommand({ + command: 'install', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'proxy', + runId: 'proxy-client-1', + session: 'adc-proxy', + platform: 'apple', + }, + client: createTestClient({ + listDevices: async () => [ + { + platform: 'ios', + target: 'mobile', + kind: 'simulator', + id: 'SIM-001', + name: 'iPhone 16', + booted: true, + identifiers: { udid: 'SIM-001' }, + ios: { udid: 'SIM-001' }, + }, + ], + allocate: allocate.stub, + }), + }); + + assert.equal(allocate.request?.deviceKey, 'ios:mobile:SIM-001'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// The comparison itself, without a device resolution in the way: the connection bound `ios` from +// `connect`, and `--platform apple` names those same devices. String inequality refused the command +// and told the user to reconnect (#2962), while `apple` versus a non-Apple platform must still +// refuse. +test('remote command with the apple family selector matches an ios-bound connection', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-apple-selector-scope-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-apple', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'acme', + runId: 'run-123', + leaseId: 'apple-lease-1', + leaseBackend: 'ios-instance', + platform: 'ios', + }, + }); + const heartbeats: string[] = []; + + const materialized = await materializeRemoteConnectionForCommand({ + command: 'snapshot', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'acme', + runId: 'run-123', + session: 'adc-apple', + platform: 'apple', + }, + client: createTestClient({ + heartbeat: async (request) => { + heartbeats.push(request.leaseId); + return { + leaseId: request.leaseId, + tenantId: 'acme', + runId: 'run-123', + backend: 'ios-instance', + }; + }, + }), + }); + + assert.deepEqual(heartbeats, ['apple-lease-1']); + assert.equal(materialized.flags.leaseId, 'apple-lease-1'); + assert.equal(materialized.flags.platform, 'ios'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('proxy install against a differently-bound platform is still refused', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-proxy-platform-conflict-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-proxy', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'proxy', + runId: 'proxy-client-1', + leaseProvider: 'proxy', + clientId: 'client-1', + platform: 'ios', + leaseBackend: 'ios-instance', + }, + }); + + await assert.rejects( + async () => + await materializeRemoteConnectionForCommand({ + command: 'install', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'proxy', + runId: 'proxy-client-1', + session: 'adc-proxy', + platform: 'android', + }, + client: createTestClient(), + }), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.details?.session === 'adc-proxy' && + error.details?.platform === 'ios', + ); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// The resolved device's internal platform is the collapsed `apple`, while the lease request, the +// connection state, and `--platform` all speak the public leaf `ios`. Writing the internal value +// into the connection state is what #2962 reported: it refused the next command of the same +// session, whose bound platform was read back out of that state. +test('proxy install records the public platform and the next command reuses that scope', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-proxy-ios-install-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-proxy', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'proxy', + runId: 'proxy-client-1', + leaseProvider: 'proxy', + clientId: 'client-1', + }, + }); + const allocate = recordedLeaseAllocate({ leaseId: 'ios-lease-1', backend: 'ios-instance' }); + const client = createTestClient({ + listDevices: async () => [ + { + platform: 'ios', + target: 'mobile', + kind: 'simulator', + id: 'SIM-001', + name: 'iPhone 16', + booted: true, + identifiers: { udid: 'SIM-001' }, + ios: { udid: 'SIM-001' }, + }, + ], + allocate: allocate.stub, + }); + const install = () => + materializeRemoteConnectionForCommand({ + command: 'install', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'proxy', + runId: 'proxy-client-1', + session: 'adc-proxy', + platform: 'ios', + }, + client, + }); + + const materialized = await install(); + assert.equal(materialized.flags.leaseId, 'ios-lease-1'); + assert.equal(materialized.flags.platform, 'ios'); + assert.equal(materialized.flags.udid, 'SIM-001'); + assert.equal(allocate.request?.platform, 'ios'); + assert.equal(allocate.request?.deviceKey, 'ios:mobile:SIM-001'); + + const state = readRemoteConnectionState({ stateDir, session: 'adc-proxy' }); + assert.equal(state?.platform, 'ios'); + assert.equal(state?.deviceKey, 'ios:mobile:SIM-001'); + + // The second command reads its bound platform back out of that state. + const reused = await install(); + assert.equal(reused.flags.platform, 'ios'); + assert.equal(reused.flags.leaseId, 'ios-lease-1'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// A connection opened with the `apple` family selector records that alias before any device is +// bound. Once a command resolves one specific device, the record must collapse to that device's +// leaf: keeping the alias would let a later command name the OTHER leaf of the same family — +// macOS against an iOS-bound lease — pass the scope guard, because family and leaf never +// conflict, and take the device's lease under a selector that names a different machine. +test('a connection opened on the apple family collapses to the bound device leaf', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-proxy-apple-family-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-proxy', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'proxy', + runId: 'proxy-client-1', + leaseProvider: 'proxy', + clientId: 'client-1', + platform: 'apple', + leaseBackend: 'ios-instance', + }, + }); + const allocate = recordedLeaseAllocate({ leaseId: 'ios-lease-1', backend: 'ios-instance' }); + const client = createTestClient({ + listDevices: async () => [ + { + platform: 'ios', + target: 'mobile', + kind: 'simulator', + id: 'SIM-001', + name: 'iPhone 16', + booted: true, + identifiers: { udid: 'SIM-001' }, + ios: { udid: 'SIM-001' }, + }, + { + platform: 'macos', + target: 'desktop', + kind: 'device', + id: 'MAC-1', + name: 'Mac', + booted: true, + identifiers: {}, + }, + ], + allocate: allocate.stub, + }); + const install = (platform: 'apple' | 'ios' | 'macos') => + materializeRemoteConnectionForCommand({ + command: 'install', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'proxy', + runId: 'proxy-client-1', + session: 'adc-proxy', + platform, + }, + client, + }); + + const materialized = await install('ios'); + assert.equal(materialized.flags.leaseId, 'ios-lease-1'); + assert.equal(materialized.flags.udid, 'SIM-001'); + + const state = readRemoteConnectionState({ stateDir, session: 'adc-proxy' }); + assert.equal(state?.platform, 'ios'); + assert.equal(state?.deviceKey, 'ios:mobile:SIM-001'); + + // macOS shares the `apple` family with the bound iOS simulator, so only the collapsed leaf + // above — not the family alias — refuses this request before it touches the lease. + await assert.rejects( + async () => await install('macos'), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.details?.session === 'adc-proxy' && + error.details?.platform === 'ios', + ); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// The same rule on the policies that never resolve a device themselves. A `connect --platform apple +// --lease-backend ios-instance` records the alias next to a backend that already rents only iOS +// devices, and this is the one command that turns it into a leaf: without the collapse the next +// `--platform macos` passed the guard, and its request went to the daemon as `apple` against the +// `ios-instance` lease instead of being refused here. +test('a default-policy connection collapses its recorded apple alias when the lease is bound', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-default-apple-binding-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-default', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'acme', + runId: 'run-9', + leaseBackend: 'ios-instance', + platform: 'apple', + }, + }); + const allocate = recordedLeaseAllocate({ leaseId: 'default-lease-1', backend: 'ios-instance' }); + const command = (platform: 'apple' | 'ios' | 'macos') => + materializeRemoteConnectionForCommand({ + command: 'snapshot', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'acme', + runId: 'run-9', + session: 'adc-default', + platform, + }, + client: createTestClient({ allocate: allocate.stub }), + }); + + const materialized = await command('apple'); + assert.equal(materialized.flags.leaseId, 'default-lease-1'); + assert.equal(materialized.flags.platform, 'ios', 'the request names the leaf it leased on'); + assert.equal(allocate.request?.platform, 'ios', 'so does the allocate payload'); + assert.equal( + readRemoteConnectionState({ stateDir, session: 'adc-default' })?.platform, + 'ios', + 'and so does the record the next command is guarded against', + ); + + await assert.rejects( + async () => await command('macos'), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.details?.platform === 'ios', + 'a second leaf of the same family has to be refused, not retargeted', + ); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// A state file written before the collapse existed records `apple` next to the backend that settled +// it, and a record that already matches its lease is never rewritten — so this connection stays as +// it was saved. The guard has to read the leaf that record owes, or the second leaf of the family +// still walks past it and the command goes out as `apple` on an `ios-instance` lease. +test('a stored apple record refuses the other leaf even though nothing rewrote it', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-apple-legacy-record-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-legacy', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'acme', + runId: 'run-9', + leaseId: 'legacy-lease-1', + leaseBackend: 'ios-instance', + deviceKey: 'ios:mobile:SIM-001', + platform: 'apple', + }, + }); + const heartbeats: string[] = []; + const command = (platform: 'macos') => + materializeRemoteConnectionForCommand({ + command: 'snapshot', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'acme', + runId: 'run-9', + session: 'adc-legacy', + platform, + }, + client: createTestClient({ + heartbeat: async (request) => { + heartbeats.push(request.leaseId); + return { + leaseId: request.leaseId, + tenantId: 'acme', + runId: 'run-9', + backend: 'ios-instance', + }; + }, + }), + }); + + await assert.rejects( + async () => await command('macos'), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.details?.platform === 'ios', + ); + assert.deepEqual(heartbeats, [], 'the lease was never touched by the refused request'); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// `connect` asks the same question a command does — is this the connection already bound? — and it +// answers it before writing anything. A record saved as `apple` next to the `ios-instance` backend +// that decided it counted as compatible with `--platform macos`, because a family and a leaf never +// conflict, so the new selector was accepted onto the iOS device's connection with no `--force`. +test('connect refuses to reuse an apple-bound connection for the other leaf', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-apple-reuse-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-apple-reuse', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'acme', + runId: 'run-9', + leaseId: 'apple-lease-1', + leaseBackend: 'ios-instance', + platform: 'apple', + }, + }); + const connect = (platform: 'apple' | 'ios' | 'macos') => + connectCommand({ + positionals: [], + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'acme', + runId: 'run-9', + session: 'adc-apple-reuse', + platform, + leaseBackend: 'ios-instance', + }, + client: createTestClient(), + }); + + // `connect` asks whether this is the same connection before it binds anything, so it refuses with + // its own "different connection, needs --force" rather than the platform conflict a command that + // does bind raises. Both refuse; only one names the axis. + await assert.rejects( + async () => await connect('macos'), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.details?.session === 'adc-apple-reuse' && + error.details?.remoteConfig === remoteConfigPath, + "the other leaf of the family can't ride along on this lease", + ); + await connect('ios'); + assert.equal( + readRemoteConnectionState({ stateDir, session: 'adc-apple-reuse' })?.platform, + 'ios', + 'the leaf the backend rents is what the reconnected record carries', + ); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// `connect` writes the platform it is asked for, which let a reused connection widen its own record +// back to the family: an `ios` lease re-declared with `--platform apple` recorded `apple`, and the +// next command could then ask for any Apple leaf and be served on the iOS device that lease holds. +// The family and the leaf name the same device here, so nothing conflicts — the record simply has to +// keep the leaf it already had. +test('connect keeps the bound leaf when re-declared with the family selector', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-apple-redeclare-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-redeclare', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'acme', + runId: 'run-9', + leaseId: 'ios-lease-1', + leaseBackend: 'ios-instance', + platform: 'ios', + }, + }); + + await connectCommand({ + positionals: [], + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'acme', + runId: 'run-9', + session: 'adc-redeclare', + platform: 'apple', + leaseBackend: 'ios-instance', + }, + client: createTestClient(), + }); + + assert.equal( + readRemoteConnectionState({ stateDir, session: 'adc-redeclare' })?.platform, + 'ios', + 'the family selector restates the connection, it does not widen it', + ); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// The two shapes where nothing in the record settles the Apple leaf, so the answer has to come from +// the backend or the request and never from the wider of the two. +// +// - A runner-guard backend (`ios-simulator`) rents no platform, so the requested leaf is the only +// leaf on the table and the recorded alias must not win it. +// - A backend that rents iOS instances has decided the leaf even when the record never wrote one, and +// that is a device this request cannot have. +test('an undecided record narrows to the requested leaf or refuses, and never sends apple', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-undecided-leaf-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + const allocations: ReturnType[] = []; + const command = ( + session: string, + leaseBackend: 'ios-simulator' | 'ios-instance', + platform: 'apple' | 'macos', + ) => { + const allocation = recordedLeaseAllocate({ + leaseId: 'undecided-lease-1', + backend: leaseBackend, + }); + allocations.push(allocation); + return materializeRemoteConnectionForCommand({ + command: 'snapshot', + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'acme', + runId: 'run-9', + session, + platform, + }, + client: createTestClient({ allocate: allocation.stub }), + }); + }; + + seedConnectionState({ + stateDir, + state: { + session: 'adc-guard', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'acme', + runId: 'run-9', + leaseBackend: 'ios-simulator', + platform: 'apple', + }, + }); + const narrowed = await command('adc-guard', 'ios-simulator', 'macos'); + assert.equal( + narrowed.flags.platform, + 'macos', + 'a runner guard rents no leaf, so the request keeps its own', + ); + assert.equal( + allocations[0]?.request?.platform, + 'macos', + 'the lease request the provider sees names the leaf, not the family the record held', + ); + + seedConnectionState({ + stateDir, + state: { + session: 'adc-unplatformed', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'acme', + runId: 'run-9', + leaseBackend: 'ios-instance', + }, + }); + await assert.rejects( + async () => await command('adc-unplatformed', 'ios-instance', 'macos'), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.details?.reason === 'CONNECTION_PLATFORM_CONFLICT' && + error.details?.platform === 'ios' && + error.details?.requestedPlatform === 'macos', + 'a backend that rents iOS is a bound device, alias or no alias', + ); + assert.equal( + allocations[1]?.request, + undefined, + 'the refused request never reached the allocator with a platform to name', + ); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +// `--force` drops the previous record before the platform is decided, so the backend is the only +// thing left naming a leaf. A connect that pairs an `ios-instance` backend with `--platform macos` +// is a contradiction on the request itself, not a reuse of a bound connection, and it must not be +// written out as a connection whose record says macOS while its backend can only ever rent iOS — +// the next command on that record would then ask a macOS device of an iOS lease. +test('connect --force refuses a backend and platform that name different devices', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-force-conflict-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + + await assert.rejects( + async () => + await connectCommand({ + positionals: [], + flags: { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'acme', + runId: 'run-9', + session: 'adc-force-conflict', + force: true, + platform: 'macos', + leaseBackend: 'ios-instance', + }, + client: createTestClient(), + }), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + error.details?.reason === 'CONNECTION_PLATFORM_CONFLICT' && + error.details?.platform === 'ios' && + error.details?.requestedPlatform === 'macos', + 'a backend that rents iOS cannot be asked for macOS, --force or not', + ); + assert.equal( + readRemoteConnectionState({ stateDir, session: 'adc-force-conflict' }), + null, + 'the refused connect wrote no connection state', + ); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index 961560e226..ea980bde11 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -9,18 +9,19 @@ import { resolveRemoteConfigProfile } from '../../remote/remote-config.ts'; import { readRemoteConfigFile } from '../../remote/remote-config-core.ts'; import { deviceFieldsFromPublicPlatform, - isIosFamily, - publicPlatformString, resolveDevice, type DeviceInfo, } from '@agent-device/kernel/device'; import { shouldAgentCdpUseRemoteBridgeUrl } from './agent-cdp.ts'; import { + narrowConnectionPlatform, + buildConnectionDeviceKey, buildRemoteConnectionDaemonState, buildRemoteConnectionRequestMetadata, hashRemoteConfigFile, mergeRemoteConnectionRequestMetadata, readRemoteConnectionState, + resolveConnectionDeviceScope, writeRemoteConnectionState, type RemoteConnectionState, type RemoteConnectionRequestMetadata, @@ -30,6 +31,7 @@ import type { BatchStep } from '@agent-device/contracts/client'; import { AppError } from '@agent-device/kernel/errors'; import { isSessionRuntimePlatform, + leaseBackendForPlatform, type LeaseBackend, type SessionRuntimeHints, } from '@agent-device/kernel/contracts'; @@ -142,6 +144,27 @@ export async function materializeRemoteConnectionForCommand(options: { acquiredLeaseForCleanup = materializedLease.acquiredLeaseForCleanup; } + // A command that allocates no lease still returns flags and a record on the platform axis, and the + // record can carry an alias the request asked to narrow. Same rule as the binding path above, so a + // connection whose backend rents one leaf cannot serve another leaf simply because this command + // never reached the allocator. + const carriedPlatform = narrowConnectionPlatform({ + leaseBackend: nextState.leaseBackend, + recordedPlatform: nextState.platform, + requestedPlatform: nextFlags.platform, + }); + if (!carriedPlatform.ok) { + throw connectionPlatformConflict({ + session: state.session, + leaseBackend: nextState.leaseBackend, + boundPlatform: carriedPlatform.boundPlatform, + requestedPlatform: carriedPlatform.requestedPlatform, + detail: 'bound-connection', + }); + } + nextState = { ...nextState, platform: carriedPlatform.platform }; + nextFlags.platform = carriedPlatform.platform; + const runtimePreparation = await prepareRuntimeForCommand({ command, flags: nextFlags, @@ -318,7 +341,29 @@ async function materializeLeaseForCommand(options: { nextState.leaseBackend ?? preliminaryLeaseBackend ?? requireRequestedLeaseBackend(nextFlags, command); - assertRequestedConnectionScope(state, nextFlags, leaseBackend); + assertRequestedConnectionBackend(state, leaseBackend); + // One decision for one axis. Whichever of the backend, the record, and the request names the + // narrowest platform, that is what this command records, sends, allocates, and returns; two that + // cannot name the same device are refused here rather than resolved later by whoever read first. + const platform = narrowConnectionPlatform({ + leaseBackend, + recordedPlatform: nextState.platform, + requestedPlatform: nextFlags.platform, + }); + if (!platform.ok) { + throw connectionPlatformConflict({ + session: state.session, + leaseBackend, + boundPlatform: platform.boundPlatform, + requestedPlatform: platform.requestedPlatform, + detail: 'bound-connection', + }); + } + // Read after the platform is settled: a request that asks for another Apple leaf also asks for a + // different target, and the platform is the reason it is being refused. + assertRequestedConnectionTarget(state, nextFlags); + nextState = { ...nextState, platform: platform.platform }; + nextFlags.platform = platform.platform; const materializedLease = await allocateOrReuseLease( client, nextState, @@ -330,7 +375,6 @@ async function materializeLeaseForCommand(options: { const lease = materializedLease.lease; nextFlags.leaseId = lease.leaseId; nextFlags.leaseBackend = leaseBackend; - nextFlags.platform = nextState.platform ?? nextFlags.platform; nextFlags.target = nextState.target ?? nextFlags.target; if (leaseStateMatches(nextState, lease, leaseBackend)) { return { @@ -690,11 +734,7 @@ async function releaseAcquiredLeaseOnWriteFailure( } export function resolveRequestedLeaseBackend(flags: CliFlags): LeaseBackend | undefined { - if (flags.leaseBackend) return flags.leaseBackend; - if (flags.platform === 'android') return 'android-instance'; - if (flags.platform === 'ios') return 'ios-instance'; - if (flags.platform === 'harmonyos') return 'harmonyos-instance'; - return undefined; + return flags.leaseBackend ?? leaseBackendForPlatform(flags.platform); } function requireRequestedLeaseBackend(flags: CliFlags, command: string): LeaseBackend { @@ -870,15 +910,14 @@ async function resolveProxyLeaseState(options: { ); } const device = await resolveSelectedDevice(options.client, options.flags); - const deviceKey = buildProxyDeviceKey(device); + const scope = resolveConnectionDeviceScope(device); return { state: { ...options.state, - deviceKey, - leaseBackend: - options.state.leaseBackend ?? options.leaseBackend ?? leaseBackendForDevice(device), - platform: options.state.platform ?? device.platform, - target: options.state.target ?? device.target, + deviceKey: buildConnectionDeviceKey(scope), + leaseBackend: options.state.leaseBackend ?? options.leaseBackend ?? scope.leaseBackend, + platform: scope.platform, + target: options.state.target ?? scope.target, updatedAt: new Date().toISOString(), }, device, @@ -886,15 +925,11 @@ async function resolveProxyLeaseState(options: { } function applyResolvedDeviceSelector(flags: CliFlags, device: DeviceInfo): void { - flags.platform = device.platform; - flags.target = device.target ?? flags.target; - if (isIosFamily(device)) { - flags.udid = device.id; - return; - } - if (device.platform === 'android' || device.platform === 'harmonyos') { - flags.serial = device.id; - } + const scope = resolveConnectionDeviceScope(device); + flags.platform = scope.platform; + flags.target = scope.target ?? flags.target; + if (scope.identityFlag === 'udid') flags.udid = scope.id; + if (scope.identityFlag === 'serial') flags.serial = scope.id; } async function resolveSelectedDevice( @@ -929,20 +964,38 @@ async function resolveSelectedDevice( ); } -function buildProxyDeviceKey(device: DeviceInfo): string { - return `${publicPlatformString(device)}:${device.target ?? 'mobile'}:${device.id}`; -} - -function leaseBackendForDevice(device: DeviceInfo): LeaseBackend | undefined { - if (isIosFamily(device)) return 'ios-instance'; - if (device.platform === 'android') return 'android-instance'; - if (device.platform === 'harmonyos') return 'harmonyos-instance'; - return undefined; +/** + * The refusal raised when the platform axis cannot be decided: two of the backend, the record, and + * the request name devices that are not the same device. `detail` says which of the three disagreed, + * because the advice differs — a bound connection is replaced with `--force`, and a request that + * contradicts itself has no `--force` to reach. + */ +export function connectionPlatformConflict( + options: Readonly<{ + session: string; + leaseBackend: LeaseBackend | undefined; + boundPlatform?: CliFlags['platform']; + requestedPlatform?: CliFlags['platform']; + detail: 'bound-connection' | 'requested-backend'; + }>, +): AppError { + return new AppError( + 'INVALID_ARGS', + options.detail === 'bound-connection' + ? 'Active remote connection is already bound to a different platform. Re-run connect --force to replace it.' + : 'The requested platform does not match the device this lease backend rents.', + { + session: options.session, + leaseBackend: options.leaseBackend, + platform: options.boundPlatform, + requestedPlatform: options.requestedPlatform, + reason: 'CONNECTION_PLATFORM_CONFLICT', + }, + ); } -function assertRequestedConnectionScope( +function assertRequestedConnectionBackend( state: RemoteConnectionState, - flags: CliFlags, requestedLeaseBackend: LeaseBackend, ): void { if (state.leaseBackend && state.leaseBackend !== requestedLeaseBackend) { @@ -952,13 +1005,9 @@ function assertRequestedConnectionScope( { session: state.session, leaseBackend: state.leaseBackend }, ); } - if (state.platform && flags.platform && state.platform !== flags.platform) { - throw new AppError( - 'INVALID_ARGS', - 'Active remote connection is already bound to a different platform. Re-run connect --force to replace it.', - { session: state.session, platform: state.platform }, - ); - } +} + +function assertRequestedConnectionTarget(state: RemoteConnectionState, flags: CliFlags): void { if (state.target && flags.target && state.target !== flags.target) { throw new AppError( 'INVALID_ARGS', diff --git a/src/cli/commands/connection.ts b/src/cli/commands/connection.ts index bbef4ee2f2..9542484684 100644 --- a/src/cli/commands/connection.ts +++ b/src/cli/commands/connection.ts @@ -11,6 +11,8 @@ import { readRemoteConnectionState, remoteConnectionLeaseIdentityMatches, removeRemoteConnectionState, + connectionPlatformMatchesSelection, + narrowConnectionPlatform, writeRemoteConnectionState, type RemoteConnectionState, type RemoteConnectionRequestMetadata, @@ -27,6 +29,7 @@ import { verifyResolvedConnectProvider, } from '../connection/connect-provider-adapters.ts'; import { + connectionPlatformConflict, hasDeferredMetroConfig, releaseRemoteConnectionLease, releasePreviousLease, @@ -161,7 +164,13 @@ function buildConnectedState(options: { : null; const now = new Date().toISOString(); const leaseBinding = buildConnectionLeaseBinding(flags, previous, connectionMetadata); - const runtimeBinding = buildConnectionRuntimeBinding(flags, previous, now); + const runtimeBinding = buildConnectionRuntimeBinding( + flags, + previous, + now, + leaseBinding, + context.session, + ); return { version: 1, session: context.session, @@ -193,13 +202,37 @@ function buildConnectionLeaseBinding( }; } +/** + * Writes what a reused connection is bound to on the platform axis. + * + * `--platform apple` on a connection whose backend rents iOS instances is the family being named + * again, not a request to widen the record back to the family: once a lease is bound the leaf is the + * truth, and a record that loses it lets the next command ask for any Apple leaf and be served on + * this one (#2962). The lease binding is asked first because the backend is what decides a family. + */ function buildConnectionRuntimeBinding( flags: CliFlags, previous: RemoteConnectionState | null, now: string, + leaseBinding: Pick, + session: string, ): Pick { + const platform = narrowConnectionPlatform({ + leaseBackend: leaseBinding.leaseBackend, + recordedPlatform: previous?.platform, + requestedPlatform: flags.platform, + }); + if (!platform.ok) { + throw connectionPlatformConflict({ + session, + leaseBackend: leaseBinding.leaseBackend, + boundPlatform: platform.boundPlatform, + requestedPlatform: platform.requestedPlatform, + detail: previous ? 'bound-connection' : 'requested-backend', + }); + } return { - platform: flags.platform ?? previous?.platform, + platform: platform.platform, target: flags.target ?? previous?.target, runtime: previous?.runtime, metro: previous?.metro, @@ -424,9 +457,9 @@ function optionalConnectionFieldsMatch( state: RemoteConnectionState, options: Parameters[1], ): boolean { + if (!connectionPlatformMatchesSelection(state, options.flags.platform)) return false; const fieldsMatch = [ [state.leaseBackend, options.desiredLeaseBackend], - [state.platform, options.flags.platform], [state.target, options.flags.target], ].every(([left, right]) => right === undefined || left === right); return fieldsMatch && remoteConnectionLeaseIdentityMatches(state, options.connection); diff --git a/src/core/__tests__/device.test.ts b/src/core/__tests__/device.test.ts index a4b0f306b6..88379e7ab3 100644 --- a/src/core/__tests__/device.test.ts +++ b/src/core/__tests__/device.test.ts @@ -4,6 +4,7 @@ import { isPlatform, isTvOsDevice, matchesPlatformSelector, + platformSelectorsConflict, PLATFORMS, resolveApplePlatformName, resolveAppleSimulatorSetPathForSelector, @@ -50,6 +51,25 @@ test('matchesPlatformSelector resolves apple selector across Apple platforms', ( assert.equal(matchesPlatformSelector({ platform: 'vega' }, 'android'), false); }); +// #2962: a connection bound to `ios` was compared with the internal `apple` by string inequality, +// so every iOS remote install was refused. Selectors name a platform on two axes, and deciding +// "different platform" has to account for both. +test('platformSelectorsConflict reads selectors on both the family and leaf axes', () => { + assert.equal(platformSelectorsConflict('ios', 'apple'), false); + assert.equal(platformSelectorsConflict('apple', 'ios'), false); + assert.equal(platformSelectorsConflict('macos', 'apple'), false); + assert.equal(platformSelectorsConflict('ios', 'ios'), false); + assert.equal(platformSelectorsConflict('apple', 'apple'), false); + assert.equal(platformSelectorsConflict('ios', 'macos'), true); + assert.equal(platformSelectorsConflict('apple', 'android'), true); + assert.equal(platformSelectorsConflict('android', 'apple'), true); + assert.equal(platformSelectorsConflict('android', 'ios'), true); + assert.equal(platformSelectorsConflict('harmonyos', 'harmonyos'), false); + // A missing selector binds nothing, so it cannot conflict with anything. + assert.equal(platformSelectorsConflict(undefined, 'ios'), false); + assert.equal(platformSelectorsConflict('ios', undefined), false); +}); + test('isPlatform accepts exactly the canonical PLATFORMS tuple', () => { for (const platform of PLATFORMS) { assert.equal(isPlatform(platform), true); diff --git a/src/daemon/handlers/__tests__/install-source.test.ts b/src/daemon/handlers/__tests__/install-source.test.ts index 274a3f8848..ca21dfbcc4 100644 --- a/src/daemon/handlers/__tests__/install-source.test.ts +++ b/src/daemon/handlers/__tests__/install-source.test.ts @@ -52,13 +52,16 @@ function makeSession(device: DeviceInfo): SessionState { return { name: 'default', createdAt: Date.now(), actions: [], device }; } -function makeRequest(source: NonNullable['installSource']): DaemonRequest { +function makeRequest( + source: NonNullable['installSource'], + flags?: DaemonRequest['flags'], +): DaemonRequest { return { token: 't', session: 'default', command: 'install_source', positionals: [], - flags: {}, + flags: flags ?? {}, meta: { installSource: source }, }; } @@ -307,6 +310,75 @@ test('install_source returns the typed iOS artifact identity supplied by its run }); }); +// The session's device carries the INTERNAL `apple` platform while `--platform` names the PUBLIC +// leaf, and a remote command's device resolution writes that leaf into the flags of every install +// it dispatches (#2962). Comparing the two axes by string equality refused the install of the very +// session it targeted, and printed the internal `apple` token the public axis is not allowed to +// emit (ADR 0009). +test('install_source accepts the public leaf selector of an Apple session it is bound to', async () => { + const store = makeStore(); + const session = makeSession({ + platform: 'apple', + appleOs: 'ios', + id: 'sim-1', + name: 'iPhone', + kind: 'simulator', + booted: true, + }); + store.set(session.name, session); + const runtime = createSourceRuntime( + session.device, + async () => ({ + installablePath: '/tmp/App.app', + bundleId: 'com.example.app', + appName: 'App', + cleanup: async () => {}, + }), + async () => ({}) as never, + ); + + const response = await handleInstallFromSourceDeploymentCommand({ + req: makeRequest({ kind: 'path', path: '/tmp/App.app' }, { platform: 'ios' }), + sessionName: session.name, + sessionStore: store, + inspectFacts: runtime.inspectFacts, + bindDevice: runtime.bindDevice, + }); + + expect(response.ok).toBe(true); +}); + +test('install_source still refuses a leaf selector that names a different platform than the session', async () => { + const store = makeStore(); + const session = makeSession({ + platform: 'apple', + appleOs: 'ios', + id: 'sim-1', + name: 'iPhone', + kind: 'simulator', + booted: true, + }); + store.set(session.name, session); + const runtime = createSourceRuntime( + session.device, + async () => ({ installablePath: '/tmp/App.app', cleanup: async () => {} }), + async () => ({}) as never, + ); + + const response = await handleInstallFromSourceDeploymentCommand({ + req: makeRequest({ kind: 'path', path: '/tmp/App.app' }, { platform: 'android' }), + sessionName: session.name, + sessionStore: store, + inspectFacts: runtime.inspectFacts, + bindDevice: runtime.bindDevice, + }); + + expect(response).toMatchObject({ ok: false, error: { code: 'INVALID_ARGS' } }); + if (response.ok) return; + expect(response.error.message).toContain('bound to ios'); + expect(response.error.message).not.toContain('apple'); +}); + function createSourceRuntime( device: DeviceInfo, materializeAppSource: PlatformRuntimeOperations['materializeAppSource'], diff --git a/src/daemon/handlers/session-app-source-deployment.ts b/src/daemon/handlers/session-app-source-deployment.ts index 4ce8400339..c1ee5a432c 100644 --- a/src/daemon/handlers/session-app-source-deployment.ts +++ b/src/daemon/handlers/session-app-source-deployment.ts @@ -4,7 +4,11 @@ import type { } from '@agent-device/contracts/app-deployment-runtime'; import type { CommandFlags } from '@agent-device/contracts/command'; import { readyMaterializeAndDeployAppUse } from '@agent-device/contracts/app-deployment-runtime-plan'; -import { isIosFamily } from '@agent-device/kernel/device'; +import { + isIosFamily, + matchesPlatformSelector, + publicPlatformString, +} from '@agent-device/kernel/device'; import { AppError, normalizeError } from '@agent-device/kernel/errors'; import { cleanupRetainedMaterializedPaths, @@ -127,10 +131,10 @@ async function resolveInstallDevice( flags: DaemonRequest['flags'] | undefined, ): Promise { const requestedPlatform = normalizePlatform(flags?.platform); - if (session && requestedPlatform && session.device.platform !== requestedPlatform) { + if (session && requestedPlatform && !matchesPlatformSelector(session.device, requestedPlatform)) { throw new AppError( 'INVALID_ARGS', - `install_from_source requested platform ${requestedPlatform}, but session is bound to ${session.device.platform}`, + `install_from_source requested platform ${requestedPlatform}, but session is bound to ${publicPlatformString(session.device)}`, ); } if (!session && !requestedPlatform) { diff --git a/src/daemon/request-lock-policy.ts b/src/daemon/request-lock-policy.ts index 105233cda0..86c43f7719 100644 --- a/src/daemon/request-lock-policy.ts +++ b/src/daemon/request-lock-policy.ts @@ -9,7 +9,7 @@ import { type SessionSelectorConflictKey, } from './session-selector.ts'; import { - isApplePlatform, + platformSelectorsConflict, publicPlatformString, type PlatformSelector, } from '@agent-device/kernel/device'; @@ -216,17 +216,6 @@ function listFreshSessionConflicts( return conflicts; } -function platformSelectorsConflict( - requested: PlatformSelector | undefined, - locked: PlatformSelector | undefined, -): boolean { - if (!requested || !locked) return false; - if (requested === locked) return false; - if (requested === 'apple') return !isApplePlatform(locked); - if (locked === 'apple') return !isApplePlatform(requested); - return true; -} - function appendFreshSessionTargetConflict( conflicts: SessionSelectorConflict[], flags: CommandFlags, diff --git a/src/remote/__tests__/remote-connection-state.test.ts b/src/remote/__tests__/remote-connection-state.test.ts index 96587bca70..0f94afa38e 100644 --- a/src/remote/__tests__/remote-connection-state.test.ts +++ b/src/remote/__tests__/remote-connection-state.test.ts @@ -2,10 +2,15 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; +import type { DeviceInfo } from '@agent-device/kernel/device'; import { mkdtempForTest } from '../../__tests__/test-utils/tmp-dir.ts'; import { + narrowConnectionPlatform, + buildConnectionDeviceKey, + connectionPlatformMatchesSelection, buildRemoteConnectionDaemonState, hashRemoteConfigFile, + resolveConnectionDeviceScope, resolveRemoteConnectionDefaults, writeRemoteConnectionState, type RemoteConnectionState, @@ -18,6 +23,87 @@ import { const FAKE_DAEMON_TOKEN = 'test-not-a-real-daemon-token'; +const IOS_SIMULATOR: DeviceInfo = { + platform: 'apple', + appleOs: 'ios', + target: 'mobile', + kind: 'simulator', + id: 'SIM-001', + name: 'iPhone 16', +}; + +const ANDROID_EMULATOR: DeviceInfo = { + platform: 'android', + target: 'mobile', + kind: 'emulator', + id: 'emulator-5554', + name: 'Pixel 8', +}; + +const MACOS_HOST: DeviceInfo = { + platform: 'apple', + appleOs: 'macos', + target: 'desktop', + kind: 'device', + id: 'HOST-MAC', + name: 'Mac', +}; + +const VEGA_VVD: DeviceInfo = { + platform: 'vega', + target: 'mobile', + kind: 'emulator', + id: 'vvd-1', + name: 'Vega VVD', +}; + +// #2962: a resolved device wrote its internal `apple` platform into the fields a remote connection +// records, which speaks the public leaf, and the scope check refused every iOS install and open. +test('resolveConnectionDeviceScope names a device on the public platform axis, never the internal one', () => { + assert.equal(resolveConnectionDeviceScope(IOS_SIMULATOR).platform, 'ios'); + assert.equal(resolveConnectionDeviceScope(MACOS_HOST).platform, 'macos'); + assert.equal(resolveConnectionDeviceScope(ANDROID_EMULATOR).platform, 'android'); +}); + +test('resolveConnectionDeviceScope pairs each device with the backend and identity flag that address it', () => { + assert.deepEqual(resolveConnectionDeviceScope(IOS_SIMULATOR), { + platform: 'ios', + target: 'mobile', + leaseBackend: 'ios-instance', + identityFlag: 'udid', + id: 'SIM-001', + }); + assert.equal(resolveConnectionDeviceScope(ANDROID_EMULATOR).leaseBackend, 'android-instance'); + assert.equal(resolveConnectionDeviceScope(ANDROID_EMULATOR).identityFlag, 'serial'); + // A platform no lease backend rents cannot be bound by a remote connection, so it names no + // identity flag either: the command fails on the missing backend rather than on a `--udid` the + // daemon reads as iOS-family-only and reports as a conflict against the session being opened. + assert.deepEqual(resolveConnectionDeviceScope(MACOS_HOST), { + platform: 'macos', + target: 'desktop', + leaseBackend: undefined, + identityFlag: undefined, + id: 'HOST-MAC', + }); + assert.equal(resolveConnectionDeviceScope(VEGA_VVD).leaseBackend, undefined); + assert.equal(resolveConnectionDeviceScope(VEGA_VVD).identityFlag, undefined); +}); + +test('buildConnectionDeviceKey keys a device by its public platform and defaulted target', () => { + assert.equal( + buildConnectionDeviceKey(resolveConnectionDeviceScope(IOS_SIMULATOR)), + 'ios:mobile:SIM-001', + ); + assert.equal( + buildConnectionDeviceKey(resolveConnectionDeviceScope(MACOS_HOST)), + 'macos:desktop:HOST-MAC', + ); + assert.equal( + buildConnectionDeviceKey(resolveConnectionDeviceScope({ ...IOS_SIMULATOR, target: undefined })), + 'ios:mobile:SIM-001', + ); +}); + test('buildRemoteConnectionDaemonState does not persist the daemon auth token', () => { const daemon = buildRemoteConnectionDaemonState({ daemonBaseUrl: 'https://daemon.example.test', @@ -100,3 +186,95 @@ test('resolveRemoteConnectionDefaults falls back to the environment token', asyn assert.equal(defaults?.flags.daemonAuthToken, FAKE_DAEMON_TOKEN); }); + +// The one rule for the platform axis. A backend that rents a leaf platform has decided the family +// whether or not the record ever wrote it; a backend that names none (`ios-simulator`, a runner guard) +// decides nothing and must not invent a leaf; and a wider `apple` never beats a leaf that is already +// known, which is what stops a recorded alias from rewriting a narrower request. +test('narrowConnectionPlatform takes the narrowest candidate and refuses a real conflict', () => { + assert.deepEqual( + narrowConnectionPlatform({ leaseBackend: 'ios-instance', recordedPlatform: 'apple' }), + { ok: true, platform: 'ios' }, + 'the backend decides a family the record never collapsed', + ); + assert.deepEqual( + narrowConnectionPlatform({ + leaseBackend: 'ios-simulator', + recordedPlatform: 'apple', + requestedPlatform: 'macos', + }), + { ok: true, platform: 'macos' }, + 'a runner-guard backend invents no leaf, so the requested one narrows the alias', + ); + assert.deepEqual( + narrowConnectionPlatform({ + leaseBackend: undefined, + recordedPlatform: 'apple', + requestedPlatform: undefined, + }), + { ok: true, platform: 'apple' }, + 'with nothing to decide from, the family selection stands', + ); + assert.deepEqual( + narrowConnectionPlatform({ + leaseBackend: 'ios-instance', + recordedPlatform: undefined, + requestedPlatform: 'macos', + }), + { ok: false, boundPlatform: 'ios', requestedPlatform: 'macos' }, + 'an unplatformed record still cannot rent an iOS backend for macOS', + ); + assert.deepEqual( + narrowConnectionPlatform({ + // A `leaseBackend` reaching here can be any string an older binary left on disk, and the + // backend-to-leaf table used to be a plain object, where these resolved to inherited + // Object members — a platform nobody rents. + leaseBackend: 'constructor' as never, + recordedPlatform: 'apple', + requestedPlatform: undefined, + }), + { ok: true, platform: 'apple' }, + 'an unrecognized backend decides nothing rather than resolving to an inherited property', + ); + assert.deepEqual( + narrowConnectionPlatform({ + leaseBackend: 'android-instance', + recordedPlatform: 'ios', + requestedPlatform: undefined, + }), + { ok: false, boundPlatform: 'android', requestedPlatform: undefined }, + 'a record that disagrees with its own backend is refused by itself, not resolved by whichever field was read first', + ); +}); + +// The reuse question `connect` asks: is this the same connection? A record that still names the +// `apple` family beside an `ios-instance` backend must not answer yes to `--platform macos`, which +// is how a stored alias let a macOS request reuse an iOS device's connection. The selector rule +// alone says family-vs-leaf is no conflict, so the collapse has to be part of this answer too. +test('connectionPlatformMatchesSelection refuses the other leaf of a bound apple record', () => { + const boundIos = { platform: 'apple', leaseBackend: 'ios-instance' } as const; + assert.equal(connectionPlatformMatchesSelection(boundIos, 'macos'), false); + assert.equal( + connectionPlatformMatchesSelection(boundIos, 'apple'), + true, + 'naming the family still matches the connection it named', + ); + assert.equal( + connectionPlatformMatchesSelection(boundIos, 'ios'), + true, + 'and so does the leaf the backend rents', + ); + assert.equal( + connectionPlatformMatchesSelection(boundIos, undefined), + true, + 'a request that names no platform asks for no platform', + ); + // The other direction of #2962: an unbound record is bound to nothing, so a request naming a + // platform is a different connection rather than a match. + assert.equal(connectionPlatformMatchesSelection({ platform: undefined }, 'ios'), false); + assert.equal( + connectionPlatformMatchesSelection({ platform: 'apple' }, 'macos'), + true, + 'with no backend, nothing has decided the family and the alias stands', + ); +}); diff --git a/src/remote/remote-connection-state.ts b/src/remote/remote-connection-state.ts index 29dbba7fa2..5e5342fe5c 100644 --- a/src/remote/remote-connection-state.ts +++ b/src/remote/remote-connection-state.ts @@ -3,10 +3,24 @@ import fs from 'node:fs'; import path from 'node:path'; import { resolveRemoteConfigPath, resolveRemoteConfigProfile } from './remote-config-core.ts'; import { AppError } from '@agent-device/kernel/errors'; +import { + deviceIdentityFlag, + platformSelectorsConflict, + publicPlatformString, + type DeviceIdentityFlag, + type DeviceInfo, + type DeviceTarget, + type PublicPlatform, +} from '@agent-device/kernel/device'; import { publishFileSync } from '@agent-device/host-kit/file'; import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import type { CliFlags } from '@agent-device/contracts/command'; -import type { LeaseBackend, SessionRuntimeHints } from '@agent-device/kernel/contracts'; +import { + leaseBackendForPlatform, + platformForLeaseBackend, + type LeaseBackend, + type SessionRuntimeHints, +} from '@agent-device/kernel/contracts'; import { leaseScopeFromOptions, leaseScopeToCommandFlags, @@ -47,6 +61,133 @@ export type RemoteConnectionRequestMetadata = Pick< 'leaseProvider' | 'deviceKey' | 'clientId' >; +/** + * A resolved device projected onto the axes a remote connection records it on. + * + * A `DeviceInfo` carries the INTERNAL platform axis (`apple`, with `appleOs` as the OS + * discriminant), while every field above — `platform`, `target`, `deviceKey`, `leaseBackend` — + * speaks the PUBLIC leaf axis (`ios`/`macos`, ADR 0009). This is where the two axes meet, so those + * fields can never disagree about which axis a device was named on. Reading `device.platform` + * directly instead was #2962: an iOS device recorded `apple` while the connection held `ios`, and + * the scope check compared the two and refused every iOS install and open on a proxy lease. + * + * Each rule it composes stays with its owning module; this answers only "which device, named how, + * rented by whom". + */ +export type ConnectionDeviceScope = Readonly<{ + platform: PublicPlatform; + /** The target as the device records it; `undefined` leaves an existing selection untouched. */ + target: DeviceTarget | undefined; + /** The lease backend that rents this device, or `undefined` when no backend leases it. */ + leaseBackend: LeaseBackend | undefined; + /** + * The flag that names this device to a request, or `undefined` when it names none. + * + * Only a device a backend can rent gets one. A platform with no lease backend cannot be bound by + * a remote connection at all, so its identity is never sent — and for the macOS desktop host it + * must not be: the daemon's own selector rule reads `--udid` as an iOS-family selector and would + * report a conflict against the session this very command is opening. Such a device fails on the + * missing backend, which names the real problem, instead of on a selector it could never use. + */ + identityFlag: DeviceIdentityFlag | undefined; + id: string; +}>; + +export function resolveConnectionDeviceScope(device: DeviceInfo): ConnectionDeviceScope { + const platform = publicPlatformString(device); + const leaseBackend = leaseBackendForPlatform(platform); + return { + platform, + target: device.target, + leaseBackend, + identityFlag: leaseBackend ? deviceIdentityFlag(platform) : undefined, + id: device.id, + }; +} + +/** The `deviceKey` for a resolved device: its identity on the public platform and target axes. */ +export function buildConnectionDeviceKey(scope: ConnectionDeviceScope): string { + return `${scope.platform}:${scope.target ?? 'mobile'}:${scope.id}`; +} + +/** + * The one platform a connection command runs as: the narrowest of what its lease backend rents, what + * its record already holds, and what the command asked for. + * + * `apple` is a family selection a caller makes before any device exists, and a family never conflicts + * with a leaf. Left as three separate compares that is exactly where a request gets retargeted: a + * record saying `apple` beside an `ios-instance` lease accepts `--platform macos`, and the recorded + * value then overwrites the asked-for one, so a macOS request goes out as `apple` against the iOS + * device the lease is paying for (#2962). One rule covers all of it — a requested selector may narrow + * what a connection is bound to and can never widen it, and two candidates that cannot name the same + * device are a conflict rather than something to pick a winner from. + * + * The backend is asked first because it is the evidence a device exists behind: a backend that rents + * iOS instances has decided the family whether or not the record ever wrote it. A backend that names + * no platform — `ios-simulator`, a runner guard below device leases — decides nothing, and inventing a + * leaf there would be the same axis mistake in the other direction. + */ +export function narrowConnectionPlatform( + evidence: Readonly<{ + leaseBackend?: LeaseBackend; + recordedPlatform?: CliFlags['platform']; + requestedPlatform?: CliFlags['platform']; + }>, +): + | Readonly<{ ok: true; platform: CliFlags['platform'] }> + | Readonly<{ + ok: false; + /** What the connection is bound to, independent of the request that was refused. */ + boundPlatform?: CliFlags['platform']; + requestedPlatform?: CliFlags['platform']; + }> { + const backendPlatform = evidence.leaseBackend + ? platformForLeaseBackend(evidence.leaseBackend) + : undefined; + const bound = [backendPlatform, evidence.recordedPlatform].filter( + (candidate): candidate is NonNullable => candidate !== undefined, + ); + // The bound side settles itself first: a record and a backend that disagree about which device they + // hold is a conflict no request can be served under, whatever it asked for. + if (bound.some((left) => bound.some((right) => platformSelectorsConflict(left, right)))) { + return { ok: false, boundPlatform: bound[0], requestedPlatform: evidence.requestedPlatform }; + } + if ( + platformSelectorsConflict(evidence.requestedPlatform, bound[0] ?? evidence.recordedPlatform) + ) { + return { ok: false, boundPlatform: bound[0], requestedPlatform: evidence.requestedPlatform }; + } + const candidates = [...bound, evidence.requestedPlatform].filter( + (candidate): candidate is NonNullable => candidate !== undefined, + ); + return { + ok: true, + platform: candidates.find((candidate) => candidate !== 'apple') ?? candidates[0], + }; +} + +/** + * Whether a `--platform` selector can be served by an existing connection. + * + * A reuse decision, not a rewrite: the same narrowing the request path applies decides this too, so a + * record saved before a backend decided its family cannot make a connection look reusable for a leaf + * it does not hold. + */ +export function connectionPlatformMatchesSelection( + state: Readonly<{ + platform?: CliFlags['platform']; + leaseBackend?: LeaseBackend; + }>, + requested: CliFlags['platform'], +): boolean { + if (requested !== undefined && state.platform === undefined) return false; + return narrowConnectionPlatform({ + leaseBackend: state.leaseBackend, + recordedPlatform: state.platform, + requestedPlatform: requested, + }).ok; +} + type RemoteConnectionDefaults = { flags: Partial; runtime?: SessionRuntimeHints;