From d8e81888b0064aa5de5df4b707c9bc89296e1dc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 09:02:31 +0100 Subject: [PATCH 01/11] refactor(move): split connection-runtime by ownership seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit connection-runtime.ts crossed the 1,000-line architecture-debt line. Split it along the seams the file already had, with no behavior change: - connection-lease-policy.ts: which lease a provider's connection allocates, its TTL, and the id it names a new lease with - connection-lease-transaction.ts: the acquisition transaction — resolve the device, allocate or renew the lease, bind it to the record, and release it - connection-metro-runtime.ts: the deferred Metro runtime preparation the record carries, and the tunnel teardown - connection-previous-lease.ts: releasing the lease a reconnect replaces, with the previous endpoint's own credential - connection-runtime.ts: the materialization that sequences those three and persists the record Importers now name the module that owns each declaration. --- .../remote-connection-harmonyos.test.ts | 6 +- src/__tests__/remote-connection.test.ts | 4 +- src/cli/commands/connection-lease-policy.ts | 95 +++ .../commands/connection-lease-transaction.ts | 328 ++++++++ src/cli/commands/connection-metro-runtime.ts | 217 +++++ src/cli/commands/connection-previous-lease.ts | 148 ++++ src/cli/commands/connection-runtime.ts | 791 +----------------- src/cli/commands/connection.ts | 7 +- src/cli/connection/ended-lease.ts | 2 +- src/cli/connection/limrun-profile.ts | 2 +- src/cli/connection/proxy-profile.ts | 2 +- 11 files changed, 817 insertions(+), 785 deletions(-) create mode 100644 src/cli/commands/connection-lease-policy.ts create mode 100644 src/cli/commands/connection-lease-transaction.ts create mode 100644 src/cli/commands/connection-metro-runtime.ts create mode 100644 src/cli/commands/connection-previous-lease.ts diff --git a/src/__tests__/remote-connection-harmonyos.test.ts b/src/__tests__/remote-connection-harmonyos.test.ts index 6e2f955a71..3d0b930e8d 100644 --- a/src/__tests__/remote-connection-harmonyos.test.ts +++ b/src/__tests__/remote-connection-harmonyos.test.ts @@ -7,10 +7,8 @@ import { forceConnectFlags, seedConnectionState, } from './remote-connection.fixtures.ts'; -import { - materializeRemoteConnectionForCommand, - resolveRequestedLeaseBackend, -} from '../cli/commands/connection-runtime.ts'; +import { materializeRemoteConnectionForCommand } from '../cli/commands/connection-runtime.ts'; +import { resolveRequestedLeaseBackend } from '../cli/commands/connection-lease-transaction.ts'; import { disconnectCommand } from '../cli/commands/connection.ts'; import { readRemoteConnectionState } from '@agent-device/remote/remote-connection-state'; diff --git a/src/__tests__/remote-connection.test.ts b/src/__tests__/remote-connection.test.ts index e183ce4610..034982b929 100644 --- a/src/__tests__/remote-connection.test.ts +++ b/src/__tests__/remote-connection.test.ts @@ -29,10 +29,10 @@ import { } from './test-utils/host-bound-commands.ts'; import { writeGeneratedRemoteConfig } from '../cli/connection/generated-config.ts'; import { - hasDeferredMetroConfig, CLOUD_WEBDRIVER_REMOTE_LEASE_TTL_MS, PROXY_REMOTE_LEASE_TTL_MS, -} from '../cli/commands/connection-runtime.ts'; +} from '../cli/commands/connection-lease-policy.ts'; +import { hasDeferredMetroConfig } from '../cli/commands/connection-metro-runtime.ts'; import { stopMetroTunnel } from '@agent-device/react-native'; import { AppError } from '@agent-device/kernel/errors'; import { diff --git a/src/cli/commands/connection-lease-policy.ts b/src/cli/commands/connection-lease-policy.ts new file mode 100644 index 0000000000..f8d2156ff5 --- /dev/null +++ b/src/cli/commands/connection-lease-policy.ts @@ -0,0 +1,95 @@ +import crypto from 'node:crypto'; +import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; +import { AppError } from '@agent-device/kernel/errors'; +import type { LeaseBackend } from '@agent-device/kernel/contracts'; +import type { CliFlags } from '@agent-device/contracts/command'; +import type { AgentDeviceClient } from '@agent-device/command-surface'; +import type { RemoteConnectionState } from '@agent-device/remote/remote-connection-state'; +import { connectionProviderCapabilities } from '../connection/provider-policy.ts'; +import { resolveProxyLeaseState, type ResolvedLeaseState } from './proxy-lease-device.ts'; + +const leaseDeferredCommands = new Set([ + 'artifacts', + 'connect', + 'connection', + 'close', + 'disconnect', + 'metro', + 'session', +]); +export const PROXY_REMOTE_LEASE_TTL_MS = 5 * 60 * 1000; +export const CLOUD_WEBDRIVER_REMOTE_LEASE_TTL_MS = 10 * 60 * 1000; + +export type ConnectionLeasePolicy = { + shouldAllocate(command: string): boolean; + ttlMs(state: RemoteConnectionState): number | undefined; + /** The id this client names a new lease with; without it the daemon mints one. */ + newLeaseId?(): string; + resolveLeaseState(options: { + command: string; + client: AgentDeviceClient; + state: RemoteConnectionState; + flags: CliFlags; + leaseBackend?: LeaseBackend; + }): Promise; +}; + +export function connectionLeasePolicyForState(state: RemoteConnectionState): ConnectionLeasePolicy { + if (state.leaseBackend === 'macos-app') return MACOS_APP_CONNECTION_LEASE_POLICY; + const capabilities = connectionProviderCapabilities(state.leaseProvider); + if (capabilities.leaseKind === 'proxy') { + return PROXY_CONNECTION_LEASE_POLICY; + } + if (capabilities.supportsDeferredAppSelection) { + return DEFERRED_APP_SELECTION_CONNECTION_LEASE_POLICY; + } + if (capabilities.usesCloudWebDriverLease) { + return CLOUD_WEBDRIVER_CONNECTION_LEASE_POLICY; + } + return DEFAULT_CONNECTION_LEASE_POLICY; +} + +const DEFERRED_APP_SELECTION_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { + shouldAllocate: (command) => + command !== PUBLIC_COMMANDS.apps && !leaseDeferredCommands.has(command), + ttlMs: () => undefined, + resolveLeaseState: async (options) => ({ state: options.state }), +}; + +const DEFAULT_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { + shouldAllocate: (command) => !leaseDeferredCommands.has(command), + ttlMs: () => undefined, + resolveLeaseState: async (options) => ({ state: options.state }), +}; + +const PROXY_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { + shouldAllocate: (command) => command !== 'devices' && !leaseDeferredCommands.has(command), + ttlMs: () => PROXY_REMOTE_LEASE_TTL_MS, + // Known before the daemon allocates, so a host in front of it can name the lease with the same id. + newLeaseId: () => crypto.randomBytes(16).toString('hex'), + resolveLeaseState: resolveProxyLeaseState, +}; + +/** + * A `macos-app` lease names one app, and the host allocated it: the client uses the lease its + * remote config names as it is, and never resolves a device into a new key or allocates a lease. + */ +const MACOS_APP_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { + shouldAllocate: (command) => command !== 'devices' && !leaseDeferredCommands.has(command), + ttlMs: () => undefined, + resolveLeaseState: async ({ state }) => { + if (!state.leaseId || !state.deviceKey) { + throw new AppError( + 'INVALID_ARGS', + 'A macos-app connection needs the leaseId and deviceKey the host allocated in its remote config.', + ); + } + return { state: { ...state, platform: 'macos' } }; + }, +}; + +const CLOUD_WEBDRIVER_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { + shouldAllocate: (command) => !leaseDeferredCommands.has(command), + ttlMs: () => CLOUD_WEBDRIVER_REMOTE_LEASE_TTL_MS, + resolveLeaseState: async (options) => ({ state: options.state }), +}; diff --git a/src/cli/commands/connection-lease-transaction.ts b/src/cli/commands/connection-lease-transaction.ts new file mode 100644 index 0000000000..fc8f9ffe1d --- /dev/null +++ b/src/cli/commands/connection-lease-transaction.ts @@ -0,0 +1,328 @@ +import { + isInactiveLeaseError, + readLeaseAllocateProviderFlags, + requireLeaseDeviceSpec, +} from '@agent-device/contracts/lease-scope'; +import { + buildRemoteConnectionRequestMetadata, + mergeRemoteConnectionRequestMetadata, + narrowConnectionPlatform, + type RemoteConnectionState, +} from '@agent-device/remote/remote-connection-state'; +import { AppError } from '@agent-device/kernel/errors'; +import { leaseBackendForPlatform, type LeaseBackend } from '@agent-device/kernel/contracts'; +import type { Lease } from '@agent-device/contracts/client'; +import type { CliFlags } from '@agent-device/contracts/command'; +import type { CloudProviderSessionResult } from '@agent-device/contracts/observability'; +import type { AgentDeviceClient } from '@agent-device/command-surface'; +import type { ConnectionLeasePolicy } from './connection-lease-policy.ts'; +import { bindLeaseNamedDevice } from './proxy-lease-device.ts'; + +export async function materializeLeaseForCommand(options: { + command: string; + client: AgentDeviceClient; + state: RemoteConnectionState; + nextState: RemoteConnectionState; + nextFlags: CliFlags; + initialApp?: string; + policy: ConnectionLeasePolicy; +}): Promise<{ + state: RemoteConnectionState; + changed: boolean; + acquiredLeaseForCleanup?: Lease; +}> { + const { command, client, state, nextFlags, policy } = options; + const preliminaryLeaseBackend = state.leaseBackend ?? resolveRequestedLeaseBackend(nextFlags); + let nextState = options.nextState; + const resolvedLeaseState = await policy.resolveLeaseState({ + command, + client, + state: nextState, + flags: nextFlags, + leaseBackend: preliminaryLeaseBackend, + }); + nextState = resolvedLeaseState.state; + const leaseBackend = + nextState.leaseBackend ?? + preliminaryLeaseBackend ?? + requireRequestedLeaseBackend(nextFlags, command); + 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; + let materializedLease = await allocateOrReuseLease( + client, + nextState, + leaseBackend, + policy, + nextFlags, + options.initialApp, + resolvedLeaseState.allocateBySpec, + ); + if (resolvedLeaseState.allocateBySpec) { + const bound = await bindLeaseNamedDevice({ + client, + state: nextState, + flags: nextFlags, + leaseBackend, + materialized: materializedLease, + allocateByDeviceKey: (boundState, flags) => + allocateOrReuseLease(client, boundState, leaseBackend, policy, flags, options.initialApp), + }); + nextState = bound.state; + materializedLease = bound.materialized; + } + const lease = materializedLease.lease; + nextFlags.leaseId = lease.leaseId; + nextFlags.leaseBackend = leaseBackend; + nextFlags.target = nextState.target ?? nextFlags.target; + if (leaseStateMatches(nextState, lease, leaseBackend)) { + return { + state: nextState, + changed: false, + acquiredLeaseForCleanup: materializedLease.acquired ? lease : undefined, + }; + } + return { + state: buildMaterializedLeaseState(nextState, lease, leaseBackend, nextFlags), + changed: true, + acquiredLeaseForCleanup: materializedLease.acquired ? lease : undefined, + }; +} + +function leaseStateMatches( + state: RemoteConnectionState, + lease: Lease, + leaseBackend: LeaseBackend, +): boolean { + return ( + state.leaseId === lease.leaseId && + state.leaseBackend === leaseBackend && + state.deviceKey === (lease.deviceKey ?? state.deviceKey) + ); +} + +function buildMaterializedLeaseState( + state: RemoteConnectionState, + lease: Lease, + leaseBackend: LeaseBackend, + flags: CliFlags, +): RemoteConnectionState { + const connection = mergeRemoteConnectionRequestMetadata(lease, state); + return { + ...state, + leaseId: lease.leaseId, + leaseBackend, + ...connection, + platform: state.platform ?? flags.platform, + target: state.target ?? flags.target, + updatedAt: new Date().toISOString(), + }; +} + +/** The host allocated a `macos-app` lease and ends it itself; a tenant never releases it. */ +export function isHostAllocatedLease(state: RemoteConnectionState): boolean { + return state.leaseBackend === 'macos-app'; +} + +export async function releaseRemoteConnectionLease( + client: AgentDeviceClient, + state: RemoteConnectionState, + // The daemon bearer token is never persisted on `state` (ADR 0007); callers + // pass the token already resolved via the flag/env/CLI-session chain. + daemonAuthToken?: string, +): Promise<{ released: boolean; provider?: CloudProviderSessionResult }> { + if (!state.leaseId || isHostAllocatedLease(state)) return { released: false }; + const result = await client.leases.release({ + tenant: state.tenant, + runId: state.runId, + leaseId: state.leaseId, + leaseBackend: state.leaseBackend, + daemonBaseUrl: state.daemon?.baseUrl, + daemonAuthToken, + daemonTransport: state.daemon?.transport, + daemonServerMode: state.daemon?.serverMode, + ...buildRemoteConnectionRequestMetadata(state), + }); + return result; +} + +export async function releaseAcquiredLeaseOnWriteFailure( + client: AgentDeviceClient, + state: RemoteConnectionState, + lease: Lease | undefined, +): Promise { + if (!lease) return; + try { + const connection = mergeRemoteConnectionRequestMetadata(state, lease); + await client.leases.release({ + tenant: state.tenant, + runId: state.runId, + leaseId: lease.leaseId, + leaseBackend: state.leaseBackend ?? lease.backend, + ...connection, + }); + } catch { + // Preserve the state-write failure; cleanup is best-effort. + } +} + +export function resolveRequestedLeaseBackend(flags: CliFlags): LeaseBackend | undefined { + return flags.leaseBackend ?? leaseBackendForPlatform(flags.platform); +} + +function requireRequestedLeaseBackend(flags: CliFlags, command: string): LeaseBackend { + const leaseBackend = resolveRequestedLeaseBackend(flags); + if (leaseBackend) return leaseBackend; + throw new AppError( + 'INVALID_ARGS', + `${command} requires --platform ios|android|harmonyos or --lease-backend when the remote connection has not resolved a lease yet.`, + ); +} + +async function allocateOrReuseLease( + client: AgentDeviceClient, + state: RemoteConnectionState, + leaseBackend: LeaseBackend, + policy: ConnectionLeasePolicy, + flags: CliFlags, + initialApp?: string, + allocateBySpec?: boolean, +): Promise<{ lease: Lease; acquired: boolean }> { + const connection = buildRemoteConnectionRequestMetadata(state); + if (state.leaseId && state.leaseBackend === leaseBackend) { + const existing = await heartbeatOrAllocateLease(client, state.leaseId, { + tenant: state.tenant, + runId: state.runId, + leaseBackend, + ...connection, + ttlMs: policy.ttlMs(state), + }); + if (existing) return { lease: existing, acquired: false }; + } + if (leaseBackend === 'macos-app') { + throw new AppError('UNAUTHORIZED', 'The host-allocated macos-app lease is no longer active.', { + reason: 'LEASE_NOT_FOUND', + leaseId: state.leaseId, + hint: 'Ask the host for a new macos-app lease and update the remote config.', + }); + } + // The spec rides only a by-spec allocate, so it is added below rather than taken from the flags. + const { + osVersion: _osVersion, + imageTag: _imageTag, + physical: _physical, + ...providerFlags + } = readLeaseAllocateProviderFlags(flags); + const lease = await client.leases.allocate({ + tenant: state.tenant, + runId: state.runId, + leaseId: policy.newLeaseId?.(), + leaseBackend, + ...connection, + ...(allocateBySpec ? { deviceKey: undefined } : {}), + ttlMs: policy.ttlMs(state), + ...providerFlags, + platform: state.platform ?? flags.platform, + target: state.target ?? flags.target, + ...(allocateBySpec ? requireLeaseDeviceSpec(flags).spec : {}), + providerApp: initialApp ?? flags.providerApp, + }); + return { lease, acquired: true }; +} + +/** + * 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 assertRequestedConnectionBackend( + state: RemoteConnectionState, + requestedLeaseBackend: LeaseBackend, +): void { + if (state.leaseBackend && state.leaseBackend !== requestedLeaseBackend) { + throw new AppError( + 'INVALID_ARGS', + 'Active remote connection is already bound to a different lease backend. Re-run connect --force to replace it.', + { session: state.session, leaseBackend: state.leaseBackend }, + ); + } +} + +function assertRequestedConnectionTarget(state: RemoteConnectionState, flags: CliFlags): void { + if (state.target && flags.target && state.target !== flags.target) { + throw new AppError( + 'INVALID_ARGS', + 'Active remote connection is already bound to a different target. Re-run connect --force to replace it.', + { session: state.session, target: state.target }, + ); + } +} + +async function heartbeatOrAllocateLease( + client: AgentDeviceClient, + leaseId: string, + scope: { + tenant: string; + runId: string; + leaseBackend: LeaseBackend; + leaseProvider?: RemoteConnectionState['leaseProvider']; + clientId?: string; + deviceKey?: string; + ttlMs?: number; + }, +): Promise { + try { + return await client.leases.heartbeat({ + ...scope, + leaseId, + }); + } catch (error) { + if (isInactiveLeaseError(error)) return undefined; + throw error; + } +} diff --git a/src/cli/commands/connection-metro-runtime.ts b/src/cli/commands/connection-metro-runtime.ts new file mode 100644 index 0000000000..6a372cf58f --- /dev/null +++ b/src/cli/commands/connection-metro-runtime.ts @@ -0,0 +1,217 @@ +import { isSessionRuntimePlatform, type SessionRuntimeHints } from '@agent-device/kernel/contracts'; +import { readMetroPrepareKind, type AgentDeviceClient } from '@agent-device/command-surface'; +import { AppError } from '@agent-device/kernel/errors'; +import { stopMetroTunnel } from '@agent-device/react-native'; +import type { BatchStep } from '@agent-device/contracts/client'; +import type { CliFlags } from '@agent-device/contracts/command'; +import type { MetroBridgeScope } from '@agent-device/contracts/remote'; +import type { RemoteConnectionState } from '@agent-device/remote/remote-connection-state'; +import { shouldAgentCdpUseRemoteBridgeUrl } from './agent-cdp.ts'; + +const runtimeDeferredCommands = new Set(['open']); + +export async function prepareRuntimeForCommand(options: { + command: string; + flags: CliFlags; + client: AgentDeviceClient; + state: RemoteConnectionState; + nextState: RemoteConnectionState; + runtime?: SessionRuntimeHints; + positionals?: string[]; + batchSteps?: BatchStep[]; + forceRuntimePrepare?: boolean; +}): Promise<{ + state: RemoteConnectionState; + runtime?: SessionRuntimeHints; + changed: boolean; + metroCleanupToStop?: RemoteConnectionState['metro']; + preparedMetroCleanupOnFailure?: RemoteConnectionState['metro']; +}> { + const { command, flags, state, client } = options; + let nextState = ensureRuntimeLeaseState(options.nextState, flags); + const nextRuntime = options.runtime; + if ( + !shouldPrepareRuntimeForCommand(command, flags, options.batchSteps, options.positionals) || + !hasDeferredMetroConfig(flags) || + !shouldPrepareRuntime(options.forceRuntimePrepare, nextRuntime, flags.platform) + ) { + return { state: nextState, runtime: nextRuntime, changed: false }; + } + if (!nextState.leaseId) { + throw new AppError( + 'INVALID_ARGS', + `${command} requires a resolved remote lease before Metro runtime can be prepared.`, + ); + } + const prepared = await prepareConnectedMetro( + flags, + client, + state.remoteConfigPath, + state.session, + { + tenantId: state.tenant, + runId: state.runId, + leaseId: nextState.leaseId, + }, + ); + const replacesExistingMetroCleanup = !isSameMetroCleanup(nextState.metro, prepared.cleanup); + nextState = { + ...nextState, + runtime: prepared.runtime, + metro: prepared.cleanup, + updatedAt: new Date().toISOString(), + }; + return { + state: nextState, + runtime: prepared.runtime, + changed: true, + metroCleanupToStop: replacesExistingMetroCleanup ? options.nextState.metro : undefined, + preparedMetroCleanupOnFailure: replacesExistingMetroCleanup ? prepared.cleanup : undefined, + }; +} + +function ensureRuntimeLeaseState( + state: RemoteConnectionState, + flags: CliFlags, +): RemoteConnectionState { + if (state.leaseId || !flags.leaseId) return state; + return { + ...state, + leaseId: flags.leaseId, + leaseBackend: flags.leaseBackend, + }; +} + +function shouldPrepareRuntime( + forceRuntimePrepare: boolean | undefined, + runtime: SessionRuntimeHints | undefined, + platform: CliFlags['platform'], +): boolean { + return ( + forceRuntimePrepare === true || !runtime || !isRuntimeCompatibleWithPlatform(runtime, platform) + ); +} + +async function prepareConnectedMetro( + flags: CliFlags, + client: AgentDeviceClient, + remoteConfigPath: string, + session: string, + bridgeScope: MetroBridgeScope, +): Promise<{ + runtime?: SessionRuntimeHints; + cleanup?: NonNullable; +}> { + if (!flags.metroProjectRoot && !flags.metroPublicBaseUrl && !flags.metroProxyBaseUrl) { + return {}; + } + if (flags.platform !== 'ios' && flags.platform !== 'android') { + throw new AppError( + 'INVALID_ARGS', + 'Deferred Metro preparation requires platform "ios" or "android".', + ); + } + if (!flags.metroPublicBaseUrl && !flags.metroProxyBaseUrl) { + throw new AppError( + 'INVALID_ARGS', + 'Deferred Metro preparation requires metroPublicBaseUrl or metroProxyBaseUrl when Metro settings are provided.', + ); + } + const prepared = await client.metro.prepare({ + projectRoot: flags.metroProjectRoot, + kind: readMetroPrepareKind(flags.metroKind), + publicBaseUrl: flags.metroPublicBaseUrl, + proxyBaseUrl: flags.metroProxyBaseUrl, + bearerToken: flags.metroBearerToken, + bridgeScope, + launchUrl: flags.launchUrl, + companionProfileKey: remoteConfigPath, + companionConsumerKey: session, + port: flags.metroPreparePort, + listenHost: flags.metroListenHost, + statusHost: flags.metroStatusHost, + startupTimeoutMs: flags.metroStartupTimeoutMs, + probeTimeoutMs: flags.metroProbeTimeoutMs, + reuseExisting: flags.metroNoReuseExisting ? false : undefined, + installDependenciesIfNeeded: flags.metroNoInstallDeps ? false : undefined, + runtimeFilePath: flags.metroRuntimeFile, + }); + return { + runtime: flags.platform === 'ios' ? prepared.iosRuntime : prepared.androidRuntime, + cleanup: flags.metroProxyBaseUrl + ? { + projectRoot: prepared.projectRoot, + profileKey: remoteConfigPath, + consumerKey: session, + } + : undefined, + }; +} + +export async function stopMetroCleanup( + cleanup: RemoteConnectionState['metro'] | undefined, +): Promise { + if (!cleanup) return; + try { + await stopMetroTunnel(cleanup); + } catch { + // Connection lifecycle cleanup must stay best-effort. + } +} + +function shouldPrepareRuntimeForCommand( + command: string, + flags: CliFlags, + batchSteps?: BatchStep[], + positionals: string[] = [], +): boolean { + if (command === 'cdp') { + return shouldAgentCdpUseRemoteBridgeUrl(positionals) && Boolean(flags.metroPublicBaseUrl); + } + if (runtimeDeferredCommands.has(command)) { + return true; + } + if (command !== 'batch' || !batchSteps) { + return false; + } + return batchSteps.some((step) => { + const stepCommand = step.command.trim().toLowerCase(); + return runtimeDeferredCommands.has(stepCommand) && step.runtime === undefined; + }); +} + +export function hasDeferredMetroConfig(flags: CliFlags): boolean { + const metroKind = flags.metroKind; + return Boolean( + flags.metroPublicBaseUrl || flags.metroProxyBaseUrl || flags.metroProjectRoot || metroKind, + ); +} + +function isRuntimeCompatibleWithPlatform( + runtime: SessionRuntimeHints, + platform: CliFlags['platform'], +): boolean { + if (!runtime.platform || !platform || !isSessionRuntimePlatform(platform)) { + return true; + } + return runtime.platform === platform; +} + +function isSameMetroCleanup( + left: RemoteConnectionState['metro'] | undefined, + right: RemoteConnectionState['metro'] | undefined, +): boolean { + return ( + left?.projectRoot === right?.projectRoot && + left?.profileKey === right?.profileKey && + left?.consumerKey === right?.consumerKey + ); +} + +export function selectCompatibleRuntime( + runtime: SessionRuntimeHints | undefined, + platform: CliFlags['platform'], +): SessionRuntimeHints | undefined { + if (!runtime) return undefined; + return isRuntimeCompatibleWithPlatform(runtime, platform) ? runtime : undefined; +} diff --git a/src/cli/commands/connection-previous-lease.ts b/src/cli/commands/connection-previous-lease.ts new file mode 100644 index 0000000000..639196fd67 --- /dev/null +++ b/src/cli/commands/connection-previous-lease.ts @@ -0,0 +1,148 @@ +import { + buildRemoteConnectionDaemonState, + type RemoteConnectionState, +} from '@agent-device/remote/remote-connection-state'; +import { readRemoteConfigFile } from '@agent-device/remote/remote-config-core'; +import type { AgentDeviceClient } from '@agent-device/command-surface'; +import type { PreviousLeaseReleaseNotice } from './connection-presentation.ts'; +import { + isHostAllocatedLease, + releaseRemoteConnectionLease, +} from './connection-lease-transaction.ts'; + +// A forced reconnect releases the *previous* connection's lease, which must be +// authenticated against the *previous* endpoint's own credential — never the +// new connection's token (that would send an unrelated endpoint's secret to +// an endpoint it was never issued for). See plans/007 for the full rule. +type PreviousLeaseAuthResolution = + | { canAuthenticate: true; daemonAuthToken?: string } + | { canAuthenticate: false }; + +function resolvePreviousLeaseAuth(options: { + previous: RemoteConnectionState; + nextDaemonBaseUrl?: string; + ambientDaemonAuthToken?: string; + cwd: string; + env: Record; +}): PreviousLeaseAuthResolution { + const ownToken = resolvePreviousOwnDaemonAuthToken(options.previous, options.cwd, options.env); + if (ownToken) return { canAuthenticate: true, daemonAuthToken: ownToken }; + if (options.previous.daemon?.baseUrl === options.nextDaemonBaseUrl) { + // Same endpoint: the ambient credential plausibly belongs to it too. + return { canAuthenticate: true, daemonAuthToken: options.ambientDaemonAuthToken }; + } + return { canAuthenticate: false }; +} + +function resolvePreviousOwnDaemonAuthToken( + previous: RemoteConnectionState, + cwd: string, + env: Record, +): string | undefined { + try { + // readRemoteConfigFile, not resolveRemoteConfigProfile: the latter merges + // ambient environment defaults (e.g. AGENT_DEVICE_DAEMON_AUTH_TOKEN) into + // the profile, which would let the *new* connection's env-sourced token + // masquerade as a credential that provably belongs to the *previous* + // endpoint. Only a token the previous config file itself declares counts + // here; the env fallback is rule 2's job, gated on matching endpoints. + const { profile } = readRemoteConfigFile({ + configPath: previous.remoteConfigPath, + cwd, + env, + }); + if (!profile.daemonAuthToken) return undefined; + // The path alone is not provenance. `remoteConfigPath` names a file *now*, + // while the claim being made is about what that file declared when the + // previous connection was established — and a config path is routinely + // reused (edited in place, re-pointed at a second environment) between the + // two. Without this check, "connect to A from ./remote.json, re-point + // ./remote.json at B, connect --force" reads B's token as A's own and + // sends it to A during lease release: the same cross-endpoint leak the + // env-merge fix closed, arriving through the file instead. + return previousConfigStillSpeaksForPreviousEndpoint(previous, profile.daemonBaseUrl) + ? profile.daemonAuthToken + : undefined; + } catch { + // A missing/unparseable previous config is the "cannot authenticate" + // case handled by the caller, not an error to propagate here. + return undefined; + } +} + +/** + * Whether the previous connection's config file can still vouch for a token as + * belonging to the previous connection's endpoint. + * + * The file must explicitly declare the same endpoint recorded in the previous + * connection state. A matching file hash proves only that the file itself did + * not change; it does not prove that its endpoint/token were effective when + * CLI flags may have overridden them. Endpoint equality is the provenance + * boundary and also preserves the benign rotated-credential case. + * + * The endpoint comparison runs both sides through + * `buildRemoteConnectionDaemonState`, the same normalizer that produced the + * stored `daemon.baseUrl`, so it compares like with like rather than raw + * strings that differ only by a trailing slash. + * + * A file that changed and no longer declares an endpoint at all cannot vouch + * for anything: the caller then falls back to rule 2 (matching endpoints) or + * reports the lease as unreleasable, which is a warning and an orphaned lease + * — the correct price for not sending a credential somewhere it may not belong. + */ +function previousConfigStillSpeaksForPreviousEndpoint( + previous: RemoteConnectionState, + declaredDaemonBaseUrl: string | undefined, +): boolean { + const declared = buildRemoteConnectionDaemonState({ + daemonBaseUrl: declaredDaemonBaseUrl, + })?.baseUrl; + return declared !== undefined && declared === previous.daemon?.baseUrl; +} + +export async function releasePreviousLease( + client: AgentDeviceClient, + previous: RemoteConnectionState, + options: { + nextDaemonBaseUrl?: string; + ambientDaemonAuthToken?: string; + cwd: string; + env: Record; + }, +): Promise { + if (!previous.leaseId || isHostAllocatedLease(previous)) return undefined; + const auth = resolvePreviousLeaseAuth({ + previous, + nextDaemonBaseUrl: options.nextDaemonBaseUrl, + ambientDaemonAuthToken: options.ambientDaemonAuthToken, + cwd: options.cwd, + env: options.env, + }); + if (!auth.canAuthenticate) { + return buildUnreleasedPreviousLeaseNotice( + previous, + 'no credential known to belong to that endpoint was available', + ); + } + try { + await releaseRemoteConnectionLease(client, previous, auth.daemonAuthToken); + return undefined; + } catch { + // Reconnect must still succeed; surface the failure instead of hiding it. + return buildUnreleasedPreviousLeaseNotice(previous, 'the release request failed'); + } +} + +function buildUnreleasedPreviousLeaseNotice( + previous: RemoteConnectionState, + reason: string, +): PreviousLeaseReleaseNotice { + return { + status: 'unreleased', + message: + `Could not release the previous lease ${previous.leaseId} ` + + `(tenant ${previous.tenant}, run ${previous.runId}) ` + + `at ${previous.daemon?.baseUrl ?? 'its daemon'}: ${reason}. ` + + 'It was left in place — release it manually if it is still active.', + }; +} diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index 6402ff1a29..03bc5f9c09 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -1,67 +1,40 @@ import type { HostEnvironment } from '@agent-device/kernel/source-value'; -import crypto from 'node:crypto'; -import type { MetroBridgeScope } from '@agent-device/contracts/remote'; import { resolveDaemonPaths } from '@agent-device/daemon-contracts/daemon-resolution'; -import { stopMetroTunnel } from '@agent-device/react-native'; import { resolveRemoteConfigProfile } from '@agent-device/remote/remote-config'; -// Provenance-preserving file-only read (no ambient env defaults merged in) — -// see resolvePreviousOwnDaemonAuthToken below for why this must not be -// resolveRemoteConfigProfile. -import { readRemoteConfigFile } from '@agent-device/remote/remote-config-core'; -import { shouldAgentCdpUseRemoteBridgeUrl } from './agent-cdp.ts'; import { - isInactiveLeaseError, - readLeaseAllocateProviderFlags, - requireLeaseDeviceSpec, -} from '@agent-device/contracts/lease-scope'; -import { - narrowConnectionPlatform, buildRemoteConnectionDaemonState, buildRemoteConnectionRequestMetadata, hashRemoteConfigFile, - mergeRemoteConnectionRequestMetadata, + narrowConnectionPlatform, readRemoteConnectionState, writeRemoteConnectionState, - type RemoteConnectionState, type RemoteConnectionRequestMetadata, + type RemoteConnectionState, } from '@agent-device/remote/remote-connection-state'; -import { profileToCliFlags } from '../remote-config-flags.ts'; -import type { BatchStep, Lease, LeaseDevice } from '@agent-device/contracts/client'; import { AppError } from '@agent-device/kernel/errors'; -import { - isSessionRuntimePlatform, - leaseBackendForPlatform, - type LeaseBackend, - type SessionRuntimeHints, -} from '@agent-device/kernel/contracts'; -import type { CliFlags } from '@agent-device/contracts/command'; -import type { CloudProviderSessionResult } from '@agent-device/contracts/observability'; +import type { BatchStep, Lease, LeaseDevice } from '@agent-device/contracts/client'; import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; -import { readMetroPrepareKind, type AgentDeviceClient } from '@agent-device/command-surface'; +import type { SessionRuntimeHints } from '@agent-device/kernel/contracts'; +import type { CliFlags } from '@agent-device/contracts/command'; +import type { AgentDeviceClient } from '@agent-device/command-surface'; import { connectionProviderCapabilities, primeProviderConnection, } from '../connection/provider-policy.ts'; -import type { PreviousLeaseReleaseNotice } from './connection-presentation.ts'; +import { profileToCliFlags } from '../remote-config-flags.ts'; +import { connectionLeasePolicyForState } from './connection-lease-policy.ts'; import { - bindLeaseNamedDevice, - grantedLeaseDevice, - resolveProxyLeaseState, - type ResolvedLeaseState, -} from './proxy-lease-device.ts'; - -const leaseDeferredCommands = new Set([ - 'artifacts', - 'connect', - 'connection', - 'close', - 'disconnect', - 'metro', - 'session', -]); -const runtimeDeferredCommands = new Set(['open']); -export const PROXY_REMOTE_LEASE_TTL_MS = 5 * 60 * 1000; -export const CLOUD_WEBDRIVER_REMOTE_LEASE_TTL_MS = 10 * 60 * 1000; + connectionPlatformConflict, + materializeLeaseForCommand, + releaseAcquiredLeaseOnWriteFailure, + resolveRequestedLeaseBackend, +} from './connection-lease-transaction.ts'; +import { + prepareRuntimeForCommand, + selectCompatibleRuntime, + stopMetroCleanup, +} from './connection-metro-runtime.ts'; +import { grantedLeaseDevice } from './proxy-lease-device.ts'; export async function materializeRemoteConnectionForCommand(options: { host: HostEnvironment; @@ -210,88 +183,6 @@ export async function materializeRemoteConnectionForCommand(options: { }; } -async function prepareRuntimeForCommand(options: { - command: string; - flags: CliFlags; - client: AgentDeviceClient; - state: RemoteConnectionState; - nextState: RemoteConnectionState; - runtime?: SessionRuntimeHints; - positionals?: string[]; - batchSteps?: BatchStep[]; - forceRuntimePrepare?: boolean; -}): Promise<{ - state: RemoteConnectionState; - runtime?: SessionRuntimeHints; - changed: boolean; - metroCleanupToStop?: RemoteConnectionState['metro']; - preparedMetroCleanupOnFailure?: RemoteConnectionState['metro']; -}> { - const { command, flags, state, client } = options; - let nextState = ensureRuntimeLeaseState(options.nextState, flags); - const nextRuntime = options.runtime; - if ( - !shouldPrepareRuntimeForCommand(command, flags, options.batchSteps, options.positionals) || - !hasDeferredMetroConfig(flags) || - !shouldPrepareRuntime(options.forceRuntimePrepare, nextRuntime, flags.platform) - ) { - return { state: nextState, runtime: nextRuntime, changed: false }; - } - if (!nextState.leaseId) { - throw new AppError( - 'INVALID_ARGS', - `${command} requires a resolved remote lease before Metro runtime can be prepared.`, - ); - } - const prepared = await prepareConnectedMetro( - flags, - client, - state.remoteConfigPath, - state.session, - { - tenantId: state.tenant, - runId: state.runId, - leaseId: nextState.leaseId, - }, - ); - const replacesExistingMetroCleanup = !isSameMetroCleanup(nextState.metro, prepared.cleanup); - nextState = { - ...nextState, - runtime: prepared.runtime, - metro: prepared.cleanup, - updatedAt: new Date().toISOString(), - }; - return { - state: nextState, - runtime: prepared.runtime, - changed: true, - metroCleanupToStop: replacesExistingMetroCleanup ? options.nextState.metro : undefined, - preparedMetroCleanupOnFailure: replacesExistingMetroCleanup ? prepared.cleanup : undefined, - }; -} - -function ensureRuntimeLeaseState( - state: RemoteConnectionState, - flags: CliFlags, -): RemoteConnectionState { - if (state.leaseId || !flags.leaseId) return state; - return { - ...state, - leaseId: flags.leaseId, - leaseBackend: flags.leaseBackend, - }; -} - -function shouldPrepareRuntime( - forceRuntimePrepare: boolean | undefined, - runtime: SessionRuntimeHints | undefined, - platform: CliFlags['platform'], -): boolean { - return ( - forceRuntimePrepare === true || !runtime || !isRuntimeCompatibleWithPlatform(runtime, platform) - ); -} - async function persistMaterializedConnection(options: { changed: boolean; stateDir: string; @@ -317,522 +208,6 @@ async function persistMaterializedConnection(options: { await stopMetroCleanup(options.metroCleanupToStop); } -async function materializeLeaseForCommand(options: { - command: string; - client: AgentDeviceClient; - state: RemoteConnectionState; - nextState: RemoteConnectionState; - nextFlags: CliFlags; - initialApp?: string; - policy: ConnectionLeasePolicy; -}): Promise<{ - state: RemoteConnectionState; - changed: boolean; - acquiredLeaseForCleanup?: Lease; -}> { - const { command, client, state, nextFlags, policy } = options; - const preliminaryLeaseBackend = state.leaseBackend ?? resolveRequestedLeaseBackend(nextFlags); - let nextState = options.nextState; - const resolvedLeaseState = await policy.resolveLeaseState({ - command, - client, - state: nextState, - flags: nextFlags, - leaseBackend: preliminaryLeaseBackend, - }); - nextState = resolvedLeaseState.state; - const leaseBackend = - nextState.leaseBackend ?? - preliminaryLeaseBackend ?? - requireRequestedLeaseBackend(nextFlags, command); - 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; - let materializedLease = await allocateOrReuseLease( - client, - nextState, - leaseBackend, - policy, - nextFlags, - options.initialApp, - resolvedLeaseState.allocateBySpec, - ); - if (resolvedLeaseState.allocateBySpec) { - const bound = await bindLeaseNamedDevice({ - client, - state: nextState, - flags: nextFlags, - leaseBackend, - materialized: materializedLease, - allocateByDeviceKey: (boundState, flags) => - allocateOrReuseLease(client, boundState, leaseBackend, policy, flags, options.initialApp), - }); - nextState = bound.state; - materializedLease = bound.materialized; - } - const lease = materializedLease.lease; - nextFlags.leaseId = lease.leaseId; - nextFlags.leaseBackend = leaseBackend; - nextFlags.target = nextState.target ?? nextFlags.target; - if (leaseStateMatches(nextState, lease, leaseBackend)) { - return { - state: nextState, - changed: false, - acquiredLeaseForCleanup: materializedLease.acquired ? lease : undefined, - }; - } - return { - state: buildMaterializedLeaseState(nextState, lease, leaseBackend, nextFlags), - changed: true, - acquiredLeaseForCleanup: materializedLease.acquired ? lease : undefined, - }; -} - -function leaseStateMatches( - state: RemoteConnectionState, - lease: Lease, - leaseBackend: LeaseBackend, -): boolean { - return ( - state.leaseId === lease.leaseId && - state.leaseBackend === leaseBackend && - state.deviceKey === (lease.deviceKey ?? state.deviceKey) - ); -} - -function buildMaterializedLeaseState( - state: RemoteConnectionState, - lease: Lease, - leaseBackend: LeaseBackend, - flags: CliFlags, -): RemoteConnectionState { - const connection = mergeRemoteConnectionRequestMetadata(lease, state); - return { - ...state, - leaseId: lease.leaseId, - leaseBackend, - ...connection, - platform: state.platform ?? flags.platform, - target: state.target ?? flags.target, - updatedAt: new Date().toISOString(), - }; -} - -type ConnectionLeasePolicy = { - shouldAllocate(command: string): boolean; - ttlMs(state: RemoteConnectionState): number | undefined; - /** The id this client names a new lease with; without it the daemon mints one. */ - newLeaseId?(): string; - resolveLeaseState(options: { - command: string; - client: AgentDeviceClient; - state: RemoteConnectionState; - flags: CliFlags; - leaseBackend?: LeaseBackend; - }): Promise; -}; - -function connectionLeasePolicyForState(state: RemoteConnectionState): ConnectionLeasePolicy { - if (state.leaseBackend === 'macos-app') return MACOS_APP_CONNECTION_LEASE_POLICY; - const capabilities = connectionProviderCapabilities(state.leaseProvider); - if (capabilities.leaseKind === 'proxy') { - return PROXY_CONNECTION_LEASE_POLICY; - } - if (capabilities.supportsDeferredAppSelection) { - return DEFERRED_APP_SELECTION_CONNECTION_LEASE_POLICY; - } - if (capabilities.usesCloudWebDriverLease) { - return CLOUD_WEBDRIVER_CONNECTION_LEASE_POLICY; - } - return DEFAULT_CONNECTION_LEASE_POLICY; -} - -const DEFERRED_APP_SELECTION_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { - shouldAllocate: (command) => - command !== PUBLIC_COMMANDS.apps && !leaseDeferredCommands.has(command), - ttlMs: () => undefined, - resolveLeaseState: async (options) => ({ state: options.state }), -}; - -const DEFAULT_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { - shouldAllocate: (command) => !leaseDeferredCommands.has(command), - ttlMs: () => undefined, - resolveLeaseState: async (options) => ({ state: options.state }), -}; - -const PROXY_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { - shouldAllocate: (command) => command !== 'devices' && !leaseDeferredCommands.has(command), - ttlMs: () => PROXY_REMOTE_LEASE_TTL_MS, - // Known before the daemon allocates, so a host in front of it can name the lease with the same id. - newLeaseId: () => crypto.randomBytes(16).toString('hex'), - resolveLeaseState: resolveProxyLeaseState, -}; - -/** - * A `macos-app` lease names one app, and the host allocated it: the client uses the lease its - * remote config names as it is, and never resolves a device into a new key or allocates a lease. - */ -const MACOS_APP_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { - shouldAllocate: (command) => command !== 'devices' && !leaseDeferredCommands.has(command), - ttlMs: () => undefined, - resolveLeaseState: async ({ state }) => { - if (!state.leaseId || !state.deviceKey) { - throw new AppError( - 'INVALID_ARGS', - 'A macos-app connection needs the leaseId and deviceKey the host allocated in its remote config.', - ); - } - return { state: { ...state, platform: 'macos' } }; - }, -}; - -const CLOUD_WEBDRIVER_CONNECTION_LEASE_POLICY: ConnectionLeasePolicy = { - shouldAllocate: (command) => !leaseDeferredCommands.has(command), - ttlMs: () => CLOUD_WEBDRIVER_REMOTE_LEASE_TTL_MS, - resolveLeaseState: async (options) => ({ state: options.state }), -}; - -async function prepareConnectedMetro( - flags: CliFlags, - client: AgentDeviceClient, - remoteConfigPath: string, - session: string, - bridgeScope: MetroBridgeScope, -): Promise<{ - runtime?: SessionRuntimeHints; - cleanup?: NonNullable; -}> { - if (!flags.metroProjectRoot && !flags.metroPublicBaseUrl && !flags.metroProxyBaseUrl) { - return {}; - } - if (flags.platform !== 'ios' && flags.platform !== 'android') { - throw new AppError( - 'INVALID_ARGS', - 'Deferred Metro preparation requires platform "ios" or "android".', - ); - } - if (!flags.metroPublicBaseUrl && !flags.metroProxyBaseUrl) { - throw new AppError( - 'INVALID_ARGS', - 'Deferred Metro preparation requires metroPublicBaseUrl or metroProxyBaseUrl when Metro settings are provided.', - ); - } - const prepared = await client.metro.prepare({ - projectRoot: flags.metroProjectRoot, - kind: readMetroPrepareKind(flags.metroKind), - publicBaseUrl: flags.metroPublicBaseUrl, - proxyBaseUrl: flags.metroProxyBaseUrl, - bearerToken: flags.metroBearerToken, - bridgeScope, - launchUrl: flags.launchUrl, - companionProfileKey: remoteConfigPath, - companionConsumerKey: session, - port: flags.metroPreparePort, - listenHost: flags.metroListenHost, - statusHost: flags.metroStatusHost, - startupTimeoutMs: flags.metroStartupTimeoutMs, - probeTimeoutMs: flags.metroProbeTimeoutMs, - reuseExisting: flags.metroNoReuseExisting ? false : undefined, - installDependenciesIfNeeded: flags.metroNoInstallDeps ? false : undefined, - runtimeFilePath: flags.metroRuntimeFile, - }); - return { - runtime: flags.platform === 'ios' ? prepared.iosRuntime : prepared.androidRuntime, - cleanup: flags.metroProxyBaseUrl - ? { - projectRoot: prepared.projectRoot, - profileKey: remoteConfigPath, - consumerKey: session, - } - : undefined, - }; -} - -export async function stopMetroCleanup( - cleanup: RemoteConnectionState['metro'] | undefined, -): Promise { - if (!cleanup) return; - try { - await stopMetroTunnel(cleanup); - } catch { - // Connection lifecycle cleanup must stay best-effort. - } -} - -/** The host allocated a `macos-app` lease and ends it itself; a tenant never releases it. */ -export function isHostAllocatedLease(state: RemoteConnectionState): boolean { - return state.leaseBackend === 'macos-app'; -} - -export async function releaseRemoteConnectionLease( - client: AgentDeviceClient, - state: RemoteConnectionState, - // The daemon bearer token is never persisted on `state` (ADR 0007); callers - // pass the token already resolved via the flag/env/CLI-session chain. - daemonAuthToken?: string, -): Promise<{ released: boolean; provider?: CloudProviderSessionResult }> { - if (!state.leaseId || isHostAllocatedLease(state)) return { released: false }; - const result = await client.leases.release({ - tenant: state.tenant, - runId: state.runId, - leaseId: state.leaseId, - leaseBackend: state.leaseBackend, - daemonBaseUrl: state.daemon?.baseUrl, - daemonAuthToken, - daemonTransport: state.daemon?.transport, - daemonServerMode: state.daemon?.serverMode, - ...buildRemoteConnectionRequestMetadata(state), - }); - return result; -} - -// A forced reconnect releases the *previous* connection's lease, which must be -// authenticated against the *previous* endpoint's own credential — never the -// new connection's token (that would send an unrelated endpoint's secret to -// an endpoint it was never issued for). See plans/007 for the full rule. -type PreviousLeaseAuthResolution = - | { canAuthenticate: true; daemonAuthToken?: string } - | { canAuthenticate: false }; - -function resolvePreviousLeaseAuth(options: { - previous: RemoteConnectionState; - nextDaemonBaseUrl?: string; - ambientDaemonAuthToken?: string; - cwd: string; - env: Record; -}): PreviousLeaseAuthResolution { - const ownToken = resolvePreviousOwnDaemonAuthToken(options.previous, options.cwd, options.env); - if (ownToken) return { canAuthenticate: true, daemonAuthToken: ownToken }; - if (options.previous.daemon?.baseUrl === options.nextDaemonBaseUrl) { - // Same endpoint: the ambient credential plausibly belongs to it too. - return { canAuthenticate: true, daemonAuthToken: options.ambientDaemonAuthToken }; - } - return { canAuthenticate: false }; -} - -function resolvePreviousOwnDaemonAuthToken( - previous: RemoteConnectionState, - cwd: string, - env: Record, -): string | undefined { - try { - // readRemoteConfigFile, not resolveRemoteConfigProfile: the latter merges - // ambient environment defaults (e.g. AGENT_DEVICE_DAEMON_AUTH_TOKEN) into - // the profile, which would let the *new* connection's env-sourced token - // masquerade as a credential that provably belongs to the *previous* - // endpoint. Only a token the previous config file itself declares counts - // here; the env fallback is rule 2's job, gated on matching endpoints. - const { profile } = readRemoteConfigFile({ - configPath: previous.remoteConfigPath, - cwd, - env, - }); - if (!profile.daemonAuthToken) return undefined; - // The path alone is not provenance. `remoteConfigPath` names a file *now*, - // while the claim being made is about what that file declared when the - // previous connection was established — and a config path is routinely - // reused (edited in place, re-pointed at a second environment) between the - // two. Without this check, "connect to A from ./remote.json, re-point - // ./remote.json at B, connect --force" reads B's token as A's own and - // sends it to A during lease release: the same cross-endpoint leak the - // env-merge fix closed, arriving through the file instead. - return previousConfigStillSpeaksForPreviousEndpoint(previous, profile.daemonBaseUrl) - ? profile.daemonAuthToken - : undefined; - } catch { - // A missing/unparseable previous config is the "cannot authenticate" - // case handled by the caller, not an error to propagate here. - return undefined; - } -} - -/** - * Whether the previous connection's config file can still vouch for a token as - * belonging to the previous connection's endpoint. - * - * The file must explicitly declare the same endpoint recorded in the previous - * connection state. A matching file hash proves only that the file itself did - * not change; it does not prove that its endpoint/token were effective when - * CLI flags may have overridden them. Endpoint equality is the provenance - * boundary and also preserves the benign rotated-credential case. - * - * The endpoint comparison runs both sides through - * `buildRemoteConnectionDaemonState`, the same normalizer that produced the - * stored `daemon.baseUrl`, so it compares like with like rather than raw - * strings that differ only by a trailing slash. - * - * A file that changed and no longer declares an endpoint at all cannot vouch - * for anything: the caller then falls back to rule 2 (matching endpoints) or - * reports the lease as unreleasable, which is a warning and an orphaned lease - * — the correct price for not sending a credential somewhere it may not belong. - */ -function previousConfigStillSpeaksForPreviousEndpoint( - previous: RemoteConnectionState, - declaredDaemonBaseUrl: string | undefined, -): boolean { - const declared = buildRemoteConnectionDaemonState({ - daemonBaseUrl: declaredDaemonBaseUrl, - })?.baseUrl; - return declared !== undefined && declared === previous.daemon?.baseUrl; -} - -export async function releasePreviousLease( - client: AgentDeviceClient, - previous: RemoteConnectionState, - options: { - nextDaemonBaseUrl?: string; - ambientDaemonAuthToken?: string; - cwd: string; - env: Record; - }, -): Promise { - if (!previous.leaseId || isHostAllocatedLease(previous)) return undefined; - const auth = resolvePreviousLeaseAuth({ - previous, - nextDaemonBaseUrl: options.nextDaemonBaseUrl, - ambientDaemonAuthToken: options.ambientDaemonAuthToken, - cwd: options.cwd, - env: options.env, - }); - if (!auth.canAuthenticate) { - return buildUnreleasedPreviousLeaseNotice( - previous, - 'no credential known to belong to that endpoint was available', - ); - } - try { - await releaseRemoteConnectionLease(client, previous, auth.daemonAuthToken); - return undefined; - } catch { - // Reconnect must still succeed; surface the failure instead of hiding it. - return buildUnreleasedPreviousLeaseNotice(previous, 'the release request failed'); - } -} - -function buildUnreleasedPreviousLeaseNotice( - previous: RemoteConnectionState, - reason: string, -): PreviousLeaseReleaseNotice { - return { - status: 'unreleased', - message: - `Could not release the previous lease ${previous.leaseId} ` + - `(tenant ${previous.tenant}, run ${previous.runId}) ` + - `at ${previous.daemon?.baseUrl ?? 'its daemon'}: ${reason}. ` + - 'It was left in place — release it manually if it is still active.', - }; -} - -async function releaseAcquiredLeaseOnWriteFailure( - client: AgentDeviceClient, - state: RemoteConnectionState, - lease: Lease | undefined, -): Promise { - if (!lease) return; - try { - const connection = mergeRemoteConnectionRequestMetadata(state, lease); - await client.leases.release({ - tenant: state.tenant, - runId: state.runId, - leaseId: lease.leaseId, - leaseBackend: state.leaseBackend ?? lease.backend, - ...connection, - }); - } catch { - // Preserve the state-write failure; cleanup is best-effort. - } -} - -export function resolveRequestedLeaseBackend(flags: CliFlags): LeaseBackend | undefined { - return flags.leaseBackend ?? leaseBackendForPlatform(flags.platform); -} - -function requireRequestedLeaseBackend(flags: CliFlags, command: string): LeaseBackend { - const leaseBackend = resolveRequestedLeaseBackend(flags); - if (leaseBackend) return leaseBackend; - throw new AppError( - 'INVALID_ARGS', - `${command} requires --platform ios|android|harmonyos or --lease-backend when the remote connection has not resolved a lease yet.`, - ); -} - -function shouldPrepareRuntimeForCommand( - command: string, - flags: CliFlags, - batchSteps?: BatchStep[], - positionals: string[] = [], -): boolean { - if (command === 'cdp') { - return shouldAgentCdpUseRemoteBridgeUrl(positionals) && Boolean(flags.metroPublicBaseUrl); - } - if (runtimeDeferredCommands.has(command)) { - return true; - } - if (command !== 'batch' || !batchSteps) { - return false; - } - return batchSteps.some((step) => { - const stepCommand = step.command.trim().toLowerCase(); - return runtimeDeferredCommands.has(stepCommand) && step.runtime === undefined; - }); -} - -export function hasDeferredMetroConfig(flags: CliFlags): boolean { - const metroKind = flags.metroKind; - return Boolean( - flags.metroPublicBaseUrl || flags.metroProxyBaseUrl || flags.metroProjectRoot || metroKind, - ); -} - -function isRuntimeCompatibleWithPlatform( - runtime: SessionRuntimeHints, - platform: CliFlags['platform'], -): boolean { - if (!runtime.platform || !platform || !isSessionRuntimePlatform(platform)) { - return true; - } - return runtime.platform === platform; -} - -function isSameMetroCleanup( - left: RemoteConnectionState['metro'] | undefined, - right: RemoteConnectionState['metro'] | undefined, -): boolean { - return ( - left?.projectRoot === right?.projectRoot && - left?.profileKey === right?.profileKey && - left?.consumerKey === right?.consumerKey - ); -} - -function selectCompatibleRuntime( - runtime: SessionRuntimeHints | undefined, - platform: CliFlags['platform'], -): SessionRuntimeHints | undefined { - if (!runtime) return undefined; - return isRuntimeCompatibleWithPlatform(runtime, platform) ? runtime : undefined; -} - // A stored connection names its provider; before one exists, the remote config profile does. function providerOf( existingState: Pick | null | undefined, @@ -887,131 +262,3 @@ function createRemoteConnectionStateFromFlags( updatedAt: now, }; } - -async function allocateOrReuseLease( - client: AgentDeviceClient, - state: RemoteConnectionState, - leaseBackend: LeaseBackend, - policy: ConnectionLeasePolicy, - flags: CliFlags, - initialApp?: string, - allocateBySpec?: boolean, -): Promise<{ lease: Lease; acquired: boolean }> { - const connection = buildRemoteConnectionRequestMetadata(state); - if (state.leaseId && state.leaseBackend === leaseBackend) { - const existing = await heartbeatOrAllocateLease(client, state.leaseId, { - tenant: state.tenant, - runId: state.runId, - leaseBackend, - ...connection, - ttlMs: policy.ttlMs(state), - }); - if (existing) return { lease: existing, acquired: false }; - } - if (leaseBackend === 'macos-app') { - throw new AppError('UNAUTHORIZED', 'The host-allocated macos-app lease is no longer active.', { - reason: 'LEASE_NOT_FOUND', - leaseId: state.leaseId, - hint: 'Ask the host for a new macos-app lease and update the remote config.', - }); - } - // The spec rides only a by-spec allocate, so it is added below rather than taken from the flags. - const { - osVersion: _osVersion, - imageTag: _imageTag, - physical: _physical, - ...providerFlags - } = readLeaseAllocateProviderFlags(flags); - const lease = await client.leases.allocate({ - tenant: state.tenant, - runId: state.runId, - leaseId: policy.newLeaseId?.(), - leaseBackend, - ...connection, - ...(allocateBySpec ? { deviceKey: undefined } : {}), - ttlMs: policy.ttlMs(state), - ...providerFlags, - platform: state.platform ?? flags.platform, - target: state.target ?? flags.target, - ...(allocateBySpec ? requireLeaseDeviceSpec(flags).spec : {}), - providerApp: initialApp ?? flags.providerApp, - }); - return { lease, acquired: true }; -} - -/** - * 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 assertRequestedConnectionBackend( - state: RemoteConnectionState, - requestedLeaseBackend: LeaseBackend, -): void { - if (state.leaseBackend && state.leaseBackend !== requestedLeaseBackend) { - throw new AppError( - 'INVALID_ARGS', - 'Active remote connection is already bound to a different lease backend. Re-run connect --force to replace it.', - { session: state.session, leaseBackend: state.leaseBackend }, - ); - } -} - -function assertRequestedConnectionTarget(state: RemoteConnectionState, flags: CliFlags): void { - if (state.target && flags.target && state.target !== flags.target) { - throw new AppError( - 'INVALID_ARGS', - 'Active remote connection is already bound to a different target. Re-run connect --force to replace it.', - { session: state.session, target: state.target }, - ); - } -} - -async function heartbeatOrAllocateLease( - client: AgentDeviceClient, - leaseId: string, - scope: { - tenant: string; - runId: string; - leaseBackend: LeaseBackend; - leaseProvider?: RemoteConnectionState['leaseProvider']; - clientId?: string; - deviceKey?: string; - ttlMs?: number; - }, -): Promise { - try { - return await client.leases.heartbeat({ - ...scope, - leaseId, - }); - } catch (error) { - if (isInactiveLeaseError(error)) return undefined; - throw error; - } -} diff --git a/src/cli/commands/connection.ts b/src/cli/commands/connection.ts index bccfdb21d8..b83152ef6a 100644 --- a/src/cli/commands/connection.ts +++ b/src/cli/commands/connection.ts @@ -30,12 +30,11 @@ import { } from '../connection/connect-provider-adapters.ts'; import { connectionPlatformConflict, - hasDeferredMetroConfig, releaseRemoteConnectionLease, - releasePreviousLease, resolveRequestedLeaseBackend, - stopMetroCleanup, -} from './connection-runtime.ts'; +} from './connection-lease-transaction.ts'; +import { hasDeferredMetroConfig, stopMetroCleanup } from './connection-metro-runtime.ts'; +import { releasePreviousLease } from './connection-previous-lease.ts'; import { writeCommandOutput } from './shared.ts'; import { formatDeliveredArtifactLines } from '@agent-device/command-output'; import { shellQuoteIfNeeded } from '@agent-device/kernel/device-shell'; diff --git a/src/cli/connection/ended-lease.ts b/src/cli/connection/ended-lease.ts index 0415d5a12c..b0aae5faa4 100644 --- a/src/cli/connection/ended-lease.ts +++ b/src/cli/connection/ended-lease.ts @@ -7,7 +7,7 @@ import { readRemoteConnectionState, writeRemoteConnectionState, } from '@agent-device/remote/remote-connection-state'; -import { isHostAllocatedLease } from '../commands/connection-runtime.ts'; +import { isHostAllocatedLease } from '../commands/connection-lease-transaction.ts'; import { connectionProviderCapabilities } from './provider-policy.ts'; /** diff --git a/src/cli/connection/limrun-profile.ts b/src/cli/connection/limrun-profile.ts index 793fd6c5ba..40ffee25b1 100644 --- a/src/cli/connection/limrun-profile.ts +++ b/src/cli/connection/limrun-profile.ts @@ -7,7 +7,7 @@ import { rejectRefusedProviderProfileFields } from '@agent-device/contracts/prov import { LIMRUN_PROFILE_FIELDS } from '@agent-device/provider-limrun'; import { readMetroProfileFields } from './profile-fields.ts'; import { persistAndResolveGeneratedProfile } from './generated-config.ts'; -import { resolveRequestedLeaseBackend } from '../commands/connection-runtime.ts'; +import { resolveRequestedLeaseBackend } from '../commands/connection-lease-transaction.ts'; import { limrunInstanceVariables, limrunPlatformForLeaseBackend, diff --git a/src/cli/connection/proxy-profile.ts b/src/cli/connection/proxy-profile.ts index c45bb33954..364c0e8b57 100644 --- a/src/cli/connection/proxy-profile.ts +++ b/src/cli/connection/proxy-profile.ts @@ -5,7 +5,7 @@ import type { CliFlags } from '@agent-device/contracts/command'; import { type EnvMap } from '@agent-device/kernel/source-value'; import { readMetroProfileFields } from './profile-fields.ts'; import { persistAndResolveGeneratedProfile } from './generated-config.ts'; -import { resolveRequestedLeaseBackend } from '../commands/connection-runtime.ts'; +import { resolveRequestedLeaseBackend } from '../commands/connection-lease-transaction.ts'; import { buildConnectClientId } from './client-id.ts'; export function resolveProxyConnectProfile(options: { From a8b775b101919f1f8f34f4ee2db5262b9a1d54a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 09:17:24 +0100 Subject: [PATCH 02/11] fix(cli): end a remote lease allocated by a materialization that never persisted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `materializeRemoteConnectionForCommand` allocated a lease and then could fail before `persistMaterializedConnection` wrote the record that would name it — a rejected `prepareConnectedMetro` was the live path. The only release path ran when the state-file write itself failed, so the new lease was neither saved nor ended: no record named it, so `disconnect` could not reach it, and it held the device until its TTL lapsed. The call now owns a lease it allocated until persistence transfers ownership, and every earlier exit ends it. A lease the call renewed was already named by the stored record, so this path never releases it — disconnect still does. The failure that ended the command stays the thrown error. A release that failed too is recorded beside it as typed secondary evidence under `details.leaseRollback` (ADR 0010 as amended by ADR 0019), never in its place. --- .../commands/connection-lease-transaction.ts | 143 ++++++++++++-- .../connection-runtime-lease-rollback.test.ts | 185 ++++++++++++++++++ src/cli/commands/connection-runtime.ts | 114 ++++------- 3 files changed, 355 insertions(+), 87 deletions(-) create mode 100644 src/cli/commands/connection-runtime-lease-rollback.test.ts diff --git a/src/cli/commands/connection-lease-transaction.ts b/src/cli/commands/connection-lease-transaction.ts index fc8f9ffe1d..5fe13eca01 100644 --- a/src/cli/commands/connection-lease-transaction.ts +++ b/src/cli/commands/connection-lease-transaction.ts @@ -1,3 +1,4 @@ +import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; import { isInactiveLeaseError, readLeaseAllocateProviderFlags, @@ -9,16 +10,94 @@ import { narrowConnectionPlatform, type RemoteConnectionState, } from '@agent-device/remote/remote-connection-state'; -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, asAppError, errorMessage } from '@agent-device/kernel/errors'; import { leaseBackendForPlatform, type LeaseBackend } from '@agent-device/kernel/contracts'; import type { Lease } from '@agent-device/contracts/client'; import type { CliFlags } from '@agent-device/contracts/command'; import type { CloudProviderSessionResult } from '@agent-device/contracts/observability'; import type { AgentDeviceClient } from '@agent-device/command-surface'; -import type { ConnectionLeasePolicy } from './connection-lease-policy.ts'; +import { connectionProviderCapabilities } from '../connection/provider-policy.ts'; +import { + connectionLeasePolicyForState, + type ConnectionLeasePolicy, +} from './connection-lease-policy.ts'; import { bindLeaseNamedDevice } from './proxy-lease-device.ts'; -export async function materializeLeaseForCommand(options: { +/** + * Brings the connection's lease up to date for this command: renews the one the record names or + * allocates the one it does not, then settles the platform axis that decides which device that lease + * rents. `unrecordedLease` is the lease this call allocated that no persisted record names yet, so + * the caller owns it until its own write transfers ownership. + */ +export async function acquireConnectionLease(options: { + command: string; + client: AgentDeviceClient; + state: RemoteConnectionState; + nextFlags: CliFlags; + positionals?: string[]; +}): Promise<{ state: RemoteConnectionState; changed: boolean; unrecordedLease?: Lease }> { + const { command, client, state, nextFlags } = options; + const deferredAppSelection = connectionProviderCapabilities( + state.leaseProvider, + ).supportsDeferredAppSelection; + const initialApp = + deferredAppSelection && command === PUBLIC_COMMANDS.open ? options.positionals?.[0] : undefined; + if (deferredAppSelection) delete nextFlags.providerApp; + const leasePolicy = connectionLeasePolicyForState(state); + let nextState = state; + let changed = false; + let unrecordedLease: Lease | undefined; + if (leasePolicy.shouldAllocate(command)) { + const materialized = await materializeLeaseForCommand({ + command, + client, + state, + nextState, + nextFlags, + initialApp, + policy: leasePolicy, + }); + nextState = materialized.state; + changed = materialized.changed; + unrecordedLease = materialized.acquiredLeaseForCleanup; + } + nextState = { + ...nextState, + platform: carriedConnectionPlatform(state.session, nextState, nextFlags), + }; + nextFlags.platform = nextState.platform; + return { state: nextState, changed, unrecordedLease }; +} + +/** + * Settles the platform axis: a command that reached no allocator still returns flags and a record on + * this axis, and the record can carry an alias the request asked to narrow. Same rule as the binding + * path inside the allocator, so a connection whose backend rents one leaf cannot serve another leaf + * simply because this command never allocated. + */ +function carriedConnectionPlatform( + session: string, + state: RemoteConnectionState, + flags: CliFlags, +): CliFlags['platform'] { + const platform = narrowConnectionPlatform({ + leaseBackend: state.leaseBackend, + recordedPlatform: state.platform, + requestedPlatform: flags.platform, + }); + if (!platform.ok) { + throw connectionPlatformConflict({ + session, + leaseBackend: state.leaseBackend, + boundPlatform: platform.boundPlatform, + requestedPlatform: platform.requestedPlatform, + detail: 'bound-connection', + }); + } + return platform.platform; +} + +async function materializeLeaseForCommand(options: { command: string; client: AgentDeviceClient; state: RemoteConnectionState; @@ -166,26 +245,60 @@ export async function releaseRemoteConnectionLease( return result; } -export async function releaseAcquiredLeaseOnWriteFailure( - client: AgentDeviceClient, - state: RemoteConnectionState, - lease: Lease | undefined, -): Promise { - if (!lease) return; +/** + * Ends the lease this command allocated when the connection record never came to name it, and + * returns the error to throw. A lease this command renewed belongs to the record it was already + * persisted against, so this path never ends it. + * + * The failure that ended the command stays the thrown error (ADR 0010, as amended by ADR 0019), with + * its own code, message and reason intact. A release that failed too is recorded beside it as typed + * secondary evidence under `details.leaseRollback`, never in its place: the command's own failure is + * what the caller has to fix, and the lease id names what stays held until the lease lapses. + */ +export async function releaseLeaseThisCommandAllocated(options: { + client: AgentDeviceClient; + state: RemoteConnectionState; + acquiredLease: Lease | undefined; + failure: unknown; +}): Promise { + const { client, state, acquiredLease, failure } = options; + if (!acquiredLease) return failure; try { - const connection = mergeRemoteConnectionRequestMetadata(state, lease); await client.leases.release({ tenant: state.tenant, runId: state.runId, - leaseId: lease.leaseId, - leaseBackend: state.leaseBackend ?? lease.backend, - ...connection, + leaseId: acquiredLease.leaseId, + leaseBackend: state.leaseBackend ?? acquiredLease.backend, + ...mergeRemoteConnectionRequestMetadata(state, acquiredLease), }); - } catch { - // Preserve the state-write failure; cleanup is best-effort. + return failure; + } catch (error) { + return withLeaseRollbackEvidence(failure, acquiredLease.leaseId, error); } } +/** How `details.leaseRollback` reads when the rollback itself could not end the lease. */ +export const LEASE_ROLLBACK_RELEASE_FAILED_REASON = 'LEASE_RELEASE_FAILED'; + +function withLeaseRollbackEvidence( + failure: unknown, + leaseId: string, + releaseError: unknown, +): unknown { + // Coercing a non-`AppError` keeps its cause chain, and gives the evidence a details bag to land in. + const primary = asAppError(failure, 'COMMAND_FAILED'); + primary.details = { + ...primary.details, + leaseRollback: { + leaseId, + released: false, + reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, + error: errorMessage(releaseError), + }, + }; + return primary; +} + export function resolveRequestedLeaseBackend(flags: CliFlags): LeaseBackend | undefined { return flags.leaseBackend ?? leaseBackendForPlatform(flags.platform); } diff --git a/src/cli/commands/connection-runtime-lease-rollback.test.ts b/src/cli/commands/connection-runtime-lease-rollback.test.ts new file mode 100644 index 0000000000..a18455ba8f --- /dev/null +++ b/src/cli/commands/connection-runtime-lease-rollback.test.ts @@ -0,0 +1,185 @@ +import { bindTestCommandHost, createTestHostEnvironment } from '@agent-device/test-kit'; +import { afterEach, test, vi } from 'vitest'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { + connectionWorkspace, + createTestClient, + recordedLeaseRelease, + seedConnectionState, +} from '../../__tests__/remote-connection.fixtures.ts'; +import { materializeRemoteConnectionForCommand } from './connection-runtime.ts'; +import { LEASE_ROLLBACK_RELEASE_FAILED_REASON } from './connection-lease-transaction.ts'; +import { readRemoteConnectionState } from '@agent-device/remote/remote-connection-state'; +import { AppError } from '@agent-device/kernel/errors'; + +bindTestCommandHost(); + +afterEach(() => { + vi.clearAllMocks(); + vi.restoreAllMocks(); +}); + +/** + * A well-formed `metro.prepare` refusal: the daemon could not reach the Metro target. One instance + * per test, because the rollback may record secondary evidence on the error it was given. + */ +function metroPrepareFailure(): AppError { + return new AppError('COMMAND_FAILED', 'Metro target is unreachable', { + reason: 'METRO_PREPARE_FAILED', + }); +} + +/** An `open` on a fresh session whose profile carries deferred Metro config. */ +function metroFlags(stateDir: string, remoteConfigPath: string) { + return { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + daemonBaseUrl: 'https://daemon.example', + tenant: 'acme', + runId: 'run-123', + session: 'adc-android', + platform: 'android' as const, + metroPublicBaseUrl: 'https://sandbox.example.test', + metroProxyBaseUrl: 'https://proxy.example.test', + }; +} + +function materializeOnHost(options: { + stateDir: string; + remoteConfigPath: string; + client: Parameters[0]['client']; +}) { + return materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: metroFlags(options.stateDir, options.remoteConfigPath), + client: options.client, + }); +} + +test('a lease this command allocated is released when Metro preparation fails', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-metro-fail-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + const release = recordedLeaseRelease(); + const prepareFailure = metroPrepareFailure(); + + await assert.rejects( + async () => + await materializeOnHost({ + stateDir, + remoteConfigPath, + client: createTestClient({ + release: release.stub, + prepare: async () => { + throw prepareFailure; + }, + }), + }), + prepareFailure, + ); + + // The record was never written, so this call still owned the lease it allocated and must end it. + assert.equal(release.request?.leaseId, 'lease-1'); + assert.equal(release.request?.tenant, 'acme'); + assert.equal(release.request?.runId, 'run-123'); + assert.equal(release.request?.leaseBackend, 'android-instance'); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-android' }), null); + + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('a lease this command renewed is kept when Metro preparation fails', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-metro-fail-reused-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + const release = recordedLeaseRelease(); + seedConnectionState({ + stateDir, + state: { + session: 'adc-android', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'acme', + runId: 'run-123', + leaseId: 'lease-renewed', + leaseBackend: 'android-instance', + platform: 'android', + }, + }); + const prepareFailure = metroPrepareFailure(); + + await assert.rejects( + async () => + await materializeOnHost({ + stateDir, + remoteConfigPath, + client: createTestClient({ + release: release.stub, + prepare: async () => { + throw prepareFailure; + }, + }), + }), + prepareFailure, + ); + + // The stored record owned that lease before this call, so disconnect still does. + assert.equal(release.request, undefined); + assert.equal( + readRemoteConnectionState({ stateDir, session: 'adc-android' })?.leaseId, + 'lease-renewed', + ); + + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('a rollback whose release failed reports it on the Metro failure instead of masking it', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-metro-fail-release-', + ); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + const prepareFailure = metroPrepareFailure(); + const releaseFailure = new AppError('COMMAND_FAILED', 'provider rejected the release'); + + let thrown: unknown; + try { + await materializeOnHost({ + stateDir, + remoteConfigPath, + client: createTestClient({ + release: async () => { + throw releaseFailure; + }, + prepare: async () => { + throw prepareFailure; + }, + }), + }); + assert.fail('expected materialization to reject'); + } catch (error) { + thrown = error; + } + + // ADR 0010 as amended by ADR 0019: the operation's own failure stays the thrown error with its + // typed reason intact, and the cleanup failure rides beside it as structured evidence. + assert.equal(thrown, prepareFailure); + assert.ok(thrown instanceof AppError); + assert.equal(thrown.code, 'COMMAND_FAILED'); + assert.equal(thrown.message, 'Metro target is unreachable'); + assert.equal(thrown.details?.reason, 'METRO_PREPARE_FAILED'); + assert.deepEqual(thrown.details?.leaseRollback, { + leaseId: 'lease-1', + released: false, + reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, + error: 'provider rejected the release', + }); + + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index 03bc5f9c09..6340182dba 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -5,7 +5,6 @@ import { buildRemoteConnectionDaemonState, buildRemoteConnectionRequestMetadata, hashRemoteConfigFile, - narrowConnectionPlatform, readRemoteConnectionState, writeRemoteConnectionState, type RemoteConnectionRequestMetadata, @@ -13,7 +12,6 @@ import { } from '@agent-device/remote/remote-connection-state'; import { AppError } from '@agent-device/kernel/errors'; import type { BatchStep, Lease, LeaseDevice } from '@agent-device/contracts/client'; -import { PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; import type { SessionRuntimeHints } from '@agent-device/kernel/contracts'; import type { CliFlags } from '@agent-device/contracts/command'; import type { AgentDeviceClient } from '@agent-device/command-surface'; @@ -22,11 +20,9 @@ import { primeProviderConnection, } from '../connection/provider-policy.ts'; import { profileToCliFlags } from '../remote-config-flags.ts'; -import { connectionLeasePolicyForState } from './connection-lease-policy.ts'; import { - connectionPlatformConflict, - materializeLeaseForCommand, - releaseAcquiredLeaseOnWriteFailure, + acquireConnectionLease, + releaseLeaseThisCommandAllocated, resolveRequestedLeaseBackend, } from './connection-lease-transaction.ts'; import { @@ -96,77 +92,57 @@ export async function materializeRemoteConnectionForCommand(options: { remoteConfig.profile, ); const nextFlags = { ...mergedFlags, session: state.session }; - const deferredAppSelection = connectionProviderCapabilities( - state.leaseProvider, - ).supportsDeferredAppSelection; - const initialApp = - deferredAppSelection && command === PUBLIC_COMMANDS.open ? options.positionals?.[0] : undefined; - if (deferredAppSelection) delete nextFlags.providerApp; let nextRuntime = selectCompatibleRuntime(state.runtime, nextFlags.platform) ?? options.runtime; let nextState = state; let changed = !existingState; - let acquiredLeaseForCleanup: Lease | undefined; - const leasePolicy = connectionLeasePolicyForState(nextState); - if (leasePolicy.shouldAllocate(command)) { - const materializedLease = await materializeLeaseForCommand({ + // This call owns a lease it allocated from the moment the provider grants it until the persisted + // record names it, which is the one place ownership transfers. Every exit before that transfer + // ends the lease: a lease no record names is invisible to disconnect, and would hold the device + // until its TTL lapses. A lease this call renewed was already named by the stored record, so this + // path leaves it to disconnect in every case, including a failure. + let unrecordedLease: Lease | undefined; + try { + const acquired = await acquireConnectionLease({ command, client, state, - nextState, nextFlags, - initialApp, - policy: leasePolicy, + positionals: options.positionals, }); - nextState = materializedLease.state; - changed = changed || materializedLease.changed; - acquiredLeaseForCleanup = materializedLease.acquiredLeaseForCleanup; - } + nextState = acquired.state; + changed = changed || acquired.changed; + unrecordedLease = acquired.unrecordedLease; - // 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', + const runtimePreparation = await prepareRuntimeForCommand({ + command, + flags: nextFlags, + client, + state, + nextState, + runtime: nextRuntime, + positionals: options.positionals, + batchSteps: options.batchSteps, + forceRuntimePrepare: options.forceRuntimePrepare, + }); + nextState = runtimePreparation.state; + nextRuntime = runtimePreparation.runtime; + changed = changed || runtimePreparation.changed; + await persistMaterializedConnection({ + changed, + stateDir, + state: nextState, + preparedMetroCleanupOnFailure: runtimePreparation.preparedMetroCleanupOnFailure, + metroCleanupToStop: runtimePreparation.metroCleanupToStop, + }); + } catch (error) { + throw await releaseLeaseThisCommandAllocated({ + client, + state: nextState, + acquiredLease: unrecordedLease, + failure: error, }); } - nextState = { ...nextState, platform: carriedPlatform.platform }; - nextFlags.platform = carriedPlatform.platform; - - const runtimePreparation = await prepareRuntimeForCommand({ - command, - flags: nextFlags, - client, - state, - nextState, - runtime: nextRuntime, - positionals: options.positionals, - batchSteps: options.batchSteps, - forceRuntimePrepare: options.forceRuntimePrepare, - }); - nextState = runtimePreparation.state; - nextRuntime = runtimePreparation.runtime; - changed = changed || runtimePreparation.changed; - await persistMaterializedConnection({ - changed, - stateDir, - state: nextState, - client, - acquiredLeaseForCleanup, - preparedMetroCleanupOnFailure: runtimePreparation.preparedMetroCleanupOnFailure, - metroCleanupToStop: runtimePreparation.metroCleanupToStop, - }); return { flags: { @@ -187,8 +163,6 @@ async function persistMaterializedConnection(options: { changed: boolean; stateDir: string; state: RemoteConnectionState; - client: AgentDeviceClient; - acquiredLeaseForCleanup?: Lease; preparedMetroCleanupOnFailure?: RemoteConnectionState['metro']; metroCleanupToStop?: RemoteConnectionState['metro']; }): Promise { @@ -196,12 +170,8 @@ async function persistMaterializedConnection(options: { try { writeRemoteConnectionState({ stateDir: options.stateDir, state: options.state }); } catch (error) { + // The caller's rollback ends the lease this write would have taken ownership of. await stopMetroCleanup(options.preparedMetroCleanupOnFailure); - await releaseAcquiredLeaseOnWriteFailure( - options.client, - options.state, - options.acquiredLeaseForCleanup, - ); throw error; } } From b4deffa99966087e2576ed9519606bc102497222 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 09:17:25 +0100 Subject: [PATCH 03/11] chore(gates): drop the legacy line cap for connection-runtime.ts The split left every module under the 1,000-line limit, so the 1,019-line legacy cap for connection-runtime.ts has no subject. --- oxlint.config.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/oxlint.config.ts b/oxlint.config.ts index 734ac10ba3..a5568d2a0b 100644 --- a/oxlint.config.ts +++ b/oxlint.config.ts @@ -63,7 +63,6 @@ const LEGACY_FILE_LINE_CAPS = { 'test/integration/provider-scenarios/android-lifecycle.test.ts': 1227, 'packages/host-kit/src/internal/exec.ts': 1032, 'packages/react-native/src/metro/client-metro.ts': 1123, - 'src/cli/commands/connection-runtime.ts': 1019, }; export default defineConfig({ From 3df3025ce73a41330213c38a85d7c246cae2975d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 11:29:31 +0100 Subject: [PATCH 04/11] fix(cli): end only a lease this command created, from allocation to the durable record The independent review found three ownership gaps in the lease rollback. - The daemon now answers each lease_allocate with whether its request created the lease (reused on the wire, Lease.created on the client). A run's repeat allocation gets the run's existing lease back, and that lease belongs to whoever holds it: this path ends only a lease the daemon says it created. - A lease enters the new ledger the moment allocation returns, before any later validation or binding can fail, so a platform conflict after a successful allocation can no longer leak it. Binding cleanup consumes the same ownership, so a lease is never released twice. - Ownership transfers at the durable session record, not at the whole write: the commit-point callback runs between the record and the active-session pointer, so a pointer failure leaves a named lease to disconnect. A release that did not end the lease keeps it owned, and the failure path reports it beside the command's own failure as typed details.leaseRollback. --- packages/client/src/lease-client.ts | 3 + packages/contracts/src/client-lease.ts | 8 + .../sessions/handlers/__tests__/lease.test.ts | 29 +++ .../daemon/src/sessions/handlers/lease.ts | 8 +- .../remote/src/remote-connection-state.ts | 7 + .../daemon-client-lease-device-spec.test.ts | 17 ++ .../remote-connection-proxy-lease-id.test.ts | 2 + src/__tests__/remote-connection.fixtures.ts | 5 + .../commands/connection-lease-transaction.ts | 225 ++++++++++++------ .../connection-runtime-lease-rollback.test.ts | 122 +++++++++- src/cli/commands/connection-runtime.ts | 40 ++-- src/cli/commands/proxy-lease-device.ts | 57 ++--- 12 files changed, 399 insertions(+), 124 deletions(-) diff --git a/packages/client/src/lease-client.ts b/packages/client/src/lease-client.ts index 5d31cfb37f..28bd5d91b8 100644 --- a/packages/client/src/lease-client.ts +++ b/packages/client/src/lease-client.ts @@ -94,6 +94,9 @@ function normalizeLease(data: Record): Lease { leaseProvider: readOptionalString(rawLease, 'leaseProvider'), clientId: readOptionalString(rawLease, 'clientId'), deviceKey: readOptionalString(rawLease, 'deviceKey'), + // Only an allocate answers `reused`; every other read, and any daemon older than the field, + // omits it, and the caller keeps `created` unset rather than guessing who made the lease. + ...(data.reused === true ? { created: false } : data.reused === false ? { created: true } : {}), ...withLeaseDevice(readLeaseDevice(rawLease.device)), ...(rawLease.retainOnClose === true ? { retainOnClose: true } : {}), createdAt: typeof rawLease.createdAt === 'number' ? rawLease.createdAt : undefined, diff --git a/packages/contracts/src/client-lease.ts b/packages/contracts/src/client-lease.ts index eb66300acd..2f64daa625 100644 --- a/packages/contracts/src/client-lease.ts +++ b/packages/contracts/src/client-lease.ts @@ -14,6 +14,14 @@ export type Lease = { leaseProvider?: string; deviceKey?: string; clientId?: string; + /** + * What an allocate answered about this request: `false` when the daemon created the lease for it, + * `true` when it handed back a lease that already existed — renewed, or already bound to the run. + * The returned lease alone cannot tell the two apart, and a caller that ends a lease must end only + * the one its own request created. Only `leases.allocate` answers this; every other read, and any + * daemon older than this field, omits it. + */ + created?: boolean; /** * The device granted for a request that named a device spec, written by the backend that * honoured the spec. A daemon refuses a spec instead, so only such a backend sets it. diff --git a/packages/daemon/src/sessions/handlers/__tests__/lease.test.ts b/packages/daemon/src/sessions/handlers/__tests__/lease.test.ts index 297805b97e..af30bccd9f 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/lease.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/lease.test.ts @@ -80,6 +80,35 @@ test('lease_allocate answers with the lease id the request named', async () => { assert.equal(registry.listActiveLeases()[0]?.leaseId, leaseId); }); +test('lease_allocate answers whether its request created the lease or got back an existing one', async () => { + const registry = new LeaseRegistry(); + const leaseId = 'abcdef0123456789abcdef0123456789'; + const allocate = () => + handleLeaseCommands({ + host: createTestHostEnvironment(), + req: { + token: 'test-token', + session: 'default', + command: 'lease_allocate', + positionals: [], + meta: { tenantId: 'tenant-a', runId: 'run-1', leaseId }, + }, + sessionName: 'default', + sessionStore: makeSessionStore('agent-device-allocate-reused-'), + leaseRegistry: registry, + }); + + const created = await allocate(); + assert.ok(created?.ok); + assert.equal(created.data?.reused, false); + // A repeat for the same run binding gets that lease back, and the answer says this request did + // not create it — a client that might end the lease must end only the one its request created. + const repeated = await allocate(); + assert.ok(repeated?.ok); + assert.equal((repeated.data?.lease as DeviceLease | undefined)?.leaseId, leaseId); + assert.equal(repeated.data?.reused, true); +}); + test('lease_release still releases a retainOnClose lease through the provider', async () => { const registry = new LeaseRegistry(); const lease = registry.allocateLease({ diff --git a/packages/daemon/src/sessions/handlers/lease.ts b/packages/daemon/src/sessions/handlers/lease.ts index 681c843d1b..b8b8168034 100644 --- a/packages/daemon/src/sessions/handlers/lease.ts +++ b/packages/daemon/src/sessions/handlers/lease.ts @@ -141,7 +141,13 @@ export async function handleLeaseCommands(args: LeaseHandlerArgs): Promise void; }): void { const statePath = remoteConnectionStatePath({ stateDir: options.stateDir, @@ -252,6 +258,7 @@ export function writeRemoteConnectionState(options: { }); fs.mkdirSync(path.dirname(statePath), { recursive: true }); writeJsonFile(statePath, options.state); + options.onSessionRecordCommitted?.(); writeJsonFile(activeConnectionStatePath(options.stateDir), { session: options.state.session }); } diff --git a/src/__tests__/daemon-client-lease-device-spec.test.ts b/src/__tests__/daemon-client-lease-device-spec.test.ts index bf3863ef51..13514bc13f 100644 --- a/src/__tests__/daemon-client-lease-device-spec.test.ts +++ b/src/__tests__/daemon-client-lease-device-spec.test.ts @@ -67,6 +67,23 @@ test('a daemon refuses a lease allocate naming a device spec and records no leas }); assert.equal(lease.leaseId, LEASE_ID); assert.equal(lease.device, undefined); + // The answer that crosses the wire says this request created the lease, so a client that may + // end a lease can tell its own from one the daemon had already issued. + assert.equal(lease.created, true); + const repeated = await client.leases.allocate({ + tenant: 'proxy', + runId: 'run-spec', + leaseBackend: 'ios-instance', + platform: 'ios', + }); + assert.equal(repeated.leaseId, LEASE_ID); + assert.equal(repeated.created, false); + await client.leases.release({ + tenant: 'proxy', + runId: 'run-spec', + leaseId: LEASE_ID, + leaseBackend: 'ios-instance', + }); } finally { await daemon.close(); } diff --git a/src/__tests__/remote-connection-proxy-lease-id.test.ts b/src/__tests__/remote-connection-proxy-lease-id.test.ts index 6cfff6fc47..ac4386ad0b 100644 --- a/src/__tests__/remote-connection-proxy-lease-id.test.ts +++ b/src/__tests__/remote-connection-proxy-lease-id.test.ts @@ -40,6 +40,8 @@ function recordingAllocate(requests: LeaseAllocateRequest[]) { leaseProvider: request.leaseProvider, clientId: request.clientId, deviceKey: request.deviceKey ?? leased.deviceKey, + // The daemon created this lease for this request, even though its id is the daemon's own. + created: true, }; }; } diff --git a/src/__tests__/remote-connection.fixtures.ts b/src/__tests__/remote-connection.fixtures.ts index 2c875d5026..7a56d22f38 100644 --- a/src/__tests__/remote-connection.fixtures.ts +++ b/src/__tests__/remote-connection.fixtures.ts @@ -100,6 +100,8 @@ export function createTestClient( tenantId: request.tenant, runId: request.runId, backend: request.leaseBackend ?? 'android-instance', + // A daemon that creates the lease for this request says so; see Lease.created. + created: true, })), heartbeat: options.heartbeat ?? @@ -210,6 +212,7 @@ export function recordedLeaseAllocate(options: { leaseProvider: incoming.leaseProvider, clientId: incoming.clientId, deviceKey: incoming.deviceKey, + created: true, }; }, get request() { @@ -343,6 +346,8 @@ export function grantedLease(request: LeaseAllocateRequest, extra: Partial Promise<{ released: boolean }>; + +/** + * The lease one command allocated and no durable record has taken over yet — the ledger the + * materialization consults before ending anything. A command owns a lease from the moment allocate + * returns it as created by this command, and ownership ends at the first durable write that names + * it, or when a deliberate release ends it. A lease this command renewed, or that the daemon says it + * did not create, was named elsewhere first: this path never ends it, because the record or the + * other run that names it owns it through any failure here. + */ +export type LeaseLedger = { + /** Takes the lease this command created, with the release that ends it. */ + track(leaseId: string, release: LeaseRelease): void; + /** + * Ends an owned lease through its recorded release. The lease stays owned when the release did + * not end it, so the failure path reports and retries it; a release already in flight is never + * attempted twice. + */ + end(leaseId: string): Promise; + /** A durable record names the lease now: this command no longer owns it. */ + transfer(leaseId: string | undefined): void; + /** Ends every lease still owned and reports the ones that stay held beside `failure`. */ + releaseOwnedOnFailure(failure: unknown): Promise; +}; + +export function createLeaseLedger(): LeaseLedger { + const owned = new Map(); + + async function attempt( + leaseId: string, + entry: { release: LeaseRelease }, + ): Promise { + try { + const outcome = await entry.release(); + if (outcome.released) owned.delete(leaseId); + return outcome.released + ? { released: true } + : { released: false, error: 'the release did not end the lease' }; + } catch (error) { + return { released: false, error: errorMessage(error) }; + } + } + + return { + track(leaseId, release) { + owned.set(leaseId, { release, inFlight: false }); + }, + async end(leaseId) { + const entry = owned.get(leaseId); + if (entry === undefined) + return { released: false, error: 'this command never owned the lease' }; + if (entry.inFlight) + return { released: false, error: 'a release for this lease is already in flight' }; + entry.inFlight = true; + try { + return await attempt(leaseId, entry); + } finally { + entry.inFlight = false; + } + }, + transfer(leaseId) { + if (leaseId !== undefined) owned.delete(leaseId); + }, + async releaseOwnedOnFailure(failure) { + const unended: { leaseId: string; error: string }[] = []; + for (const [leaseId, entry] of Array.from(owned)) { + // An outcome this command is still waiting on is reported, not retried: the lease may or + // may not have ended, and a second release beside the first is exactly what the ledger + // exists to make impossible. + const outcome = entry.inFlight + ? { released: false, error: 'a release for this lease was still in flight' } + : await attempt(leaseId, entry); + if (!outcome.released) + unended.push({ leaseId, error: outcome.error ?? 'the release was refused' }); + } + if (unended.length === 0) return failure; + const first = unended[0]!; + // The command's own failure stays the thrown error (ADR 0010, as amended by ADR 0019), and a + // release that could not end the lease rides beside it as typed evidence naming the lease that + // stays held until its TTL lapses. + const primary = asAppError(failure, 'COMMAND_FAILED'); + primary.details = { + ...primary.details, + leaseRollback: { + leaseId: first.leaseId, + released: false, + reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, + error: first.error, + ...(unended.length > 1 + ? { additionalLeaseIds: unended.slice(1).map((entry) => entry.leaseId) } + : {}), + }, + }; + return primary; + }, + }; +} + +/** How a rollback reports a lease its release could not end, beside the command's own failure. */ +export const LEASE_ROLLBACK_RELEASE_FAILED_REASON = 'LEASE_RELEASE_FAILED'; + /** * Brings the connection's lease up to date for this command: renews the one the record names or * allocates the one it does not, then settles the platform axis that decides which device that lease - * rents. `unrecordedLease` is the lease this call allocated that no persisted record names yet, so - * the caller owns it until its own write transfers ownership. + * rents. A lease this command created enters `leaseLedger` the moment the provider grants it, before + * any validation or binding here can fail, so the caller's failure path still sees it. */ export async function acquireConnectionLease(options: { command: string; @@ -35,8 +139,9 @@ export async function acquireConnectionLease(options: { state: RemoteConnectionState; nextFlags: CliFlags; positionals?: string[]; -}): Promise<{ state: RemoteConnectionState; changed: boolean; unrecordedLease?: Lease }> { - const { command, client, state, nextFlags } = options; + leaseLedger: LeaseLedger; +}): Promise<{ state: RemoteConnectionState; changed: boolean }> { + const { command, client, state, nextFlags, leaseLedger } = options; const deferredAppSelection = connectionProviderCapabilities( state.leaseProvider, ).supportsDeferredAppSelection; @@ -46,7 +151,6 @@ export async function acquireConnectionLease(options: { const leasePolicy = connectionLeasePolicyForState(state); let nextState = state; let changed = false; - let unrecordedLease: Lease | undefined; if (leasePolicy.shouldAllocate(command)) { const materialized = await materializeLeaseForCommand({ command, @@ -56,17 +160,17 @@ export async function acquireConnectionLease(options: { nextFlags, initialApp, policy: leasePolicy, + leaseLedger, }); nextState = materialized.state; changed = materialized.changed; - unrecordedLease = materialized.acquiredLeaseForCleanup; } nextState = { ...nextState, platform: carriedConnectionPlatform(state.session, nextState, nextFlags), }; nextFlags.platform = nextState.platform; - return { state: nextState, changed, unrecordedLease }; + return { state: nextState, changed }; } /** @@ -105,12 +209,9 @@ async function materializeLeaseForCommand(options: { nextFlags: CliFlags; initialApp?: string; policy: ConnectionLeasePolicy; -}): Promise<{ - state: RemoteConnectionState; - changed: boolean; - acquiredLeaseForCleanup?: Lease; -}> { - const { command, client, state, nextFlags, policy } = options; + leaseLedger: LeaseLedger; +}): Promise<{ state: RemoteConnectionState; changed: boolean }> { + const { command, client, state, nextFlags, policy, leaseLedger } = options; const preliminaryLeaseBackend = state.leaseBackend ?? resolveRequestedLeaseBackend(nextFlags); let nextState = options.nextState; const resolvedLeaseState = await policy.resolveLeaseState({ @@ -156,6 +257,7 @@ async function materializeLeaseForCommand(options: { nextFlags, options.initialApp, resolvedLeaseState.allocateBySpec, + leaseLedger, ); if (resolvedLeaseState.allocateBySpec) { const bound = await bindLeaseNamedDevice({ @@ -164,8 +266,18 @@ async function materializeLeaseForCommand(options: { flags: nextFlags, leaseBackend, materialized: materializedLease, + leaseLedger, allocateByDeviceKey: (boundState, flags) => - allocateOrReuseLease(client, boundState, leaseBackend, policy, flags, options.initialApp), + allocateOrReuseLease( + client, + boundState, + leaseBackend, + policy, + flags, + options.initialApp, + undefined, + leaseLedger, + ), }); nextState = bound.state; materializedLease = bound.materialized; @@ -175,16 +287,11 @@ async function materializeLeaseForCommand(options: { nextFlags.leaseBackend = leaseBackend; nextFlags.target = nextState.target ?? nextFlags.target; if (leaseStateMatches(nextState, lease, leaseBackend)) { - return { - state: nextState, - changed: false, - acquiredLeaseForCleanup: materializedLease.acquired ? lease : undefined, - }; + return { state: nextState, changed: false }; } return { state: buildMaterializedLeaseState(nextState, lease, leaseBackend, nextFlags), changed: true, - acquiredLeaseForCleanup: materializedLease.acquired ? lease : undefined, }; } @@ -246,57 +353,25 @@ export async function releaseRemoteConnectionLease( } /** - * Ends the lease this command allocated when the connection record never came to name it, and - * returns the error to throw. A lease this command renewed belongs to the record it was already - * persisted against, so this path never ends it. - * - * The failure that ended the command stays the thrown error (ADR 0010, as amended by ADR 0019), with - * its own code, message and reason intact. A release that failed too is recorded beside it as typed - * secondary evidence under `details.leaseRollback`, never in its place: the command's own failure is - * what the caller has to fix, and the lease id names what stays held until the lease lapses. + * The release that ends a lease this command allocated, scoped exactly as the allocation was: the + * ledger holds it from the moment the provider grants the lease, so any later cleanup — this + * command's failure path or a binding that refuses the lease — ends the same lease the same way. */ -export async function releaseLeaseThisCommandAllocated(options: { - client: AgentDeviceClient; - state: RemoteConnectionState; - acquiredLease: Lease | undefined; - failure: unknown; -}): Promise { - const { client, state, acquiredLease, failure } = options; - if (!acquiredLease) return failure; - try { +function leaseAllocationRelease( + client: AgentDeviceClient, + state: RemoteConnectionState, + leaseBackend: LeaseBackend, + lease: Lease, +): LeaseRelease { + return async () => await client.leases.release({ tenant: state.tenant, runId: state.runId, - leaseId: acquiredLease.leaseId, - leaseBackend: state.leaseBackend ?? acquiredLease.backend, - ...mergeRemoteConnectionRequestMetadata(state, acquiredLease), + leaseId: lease.leaseId, + leaseBackend, + ...mergeRemoteConnectionRequestMetadata(lease, state), + ...(lease.deviceKey !== undefined ? { deviceKey: lease.deviceKey } : {}), }); - return failure; - } catch (error) { - return withLeaseRollbackEvidence(failure, acquiredLease.leaseId, error); - } -} - -/** How `details.leaseRollback` reads when the rollback itself could not end the lease. */ -export const LEASE_ROLLBACK_RELEASE_FAILED_REASON = 'LEASE_RELEASE_FAILED'; - -function withLeaseRollbackEvidence( - failure: unknown, - leaseId: string, - releaseError: unknown, -): unknown { - // Coercing a non-`AppError` keeps its cause chain, and gives the evidence a details bag to land in. - const primary = asAppError(failure, 'COMMAND_FAILED'); - primary.details = { - ...primary.details, - leaseRollback: { - leaseId, - released: false, - reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, - error: errorMessage(releaseError), - }, - }; - return primary; } export function resolveRequestedLeaseBackend(flags: CliFlags): LeaseBackend | undefined { @@ -312,14 +387,21 @@ function requireRequestedLeaseBackend(flags: CliFlags, command: string): LeaseBa ); } +/** + * A renewed lease is the record's own; only an allocation can bring a new one into this command, and + * only when the daemon says this request created it. A run's repeat allocation gets the run's + * existing lease back — the same answer the record-less caller of another run could get — and that + * lease belongs to whoever holds it, never to this transaction. + */ async function allocateOrReuseLease( client: AgentDeviceClient, state: RemoteConnectionState, leaseBackend: LeaseBackend, policy: ConnectionLeasePolicy, flags: CliFlags, - initialApp?: string, - allocateBySpec?: boolean, + initialApp: string | undefined, + allocateBySpec: boolean | undefined, + leaseLedger: LeaseLedger, ): Promise<{ lease: Lease; acquired: boolean }> { const connection = buildRemoteConnectionRequestMetadata(state); if (state.leaseId && state.leaseBackend === leaseBackend) { @@ -360,7 +442,12 @@ async function allocateOrReuseLease( ...(allocateBySpec ? requireLeaseDeviceSpec(flags).spec : {}), providerApp: initialApp ?? flags.providerApp, }); - return { lease, acquired: true }; + const created = lease.created === true; + if (created) { + // Ownership starts here, before any caller of this function can fail on this lease. + leaseLedger.track(lease.leaseId, leaseAllocationRelease(client, state, leaseBackend, lease)); + } + return { lease, acquired: created }; } /** diff --git a/src/cli/commands/connection-runtime-lease-rollback.test.ts b/src/cli/commands/connection-runtime-lease-rollback.test.ts index a18455ba8f..903c84c228 100644 --- a/src/cli/commands/connection-runtime-lease-rollback.test.ts +++ b/src/cli/commands/connection-runtime-lease-rollback.test.ts @@ -2,11 +2,15 @@ import { bindTestCommandHost, createTestHostEnvironment } from '@agent-device/te import { afterEach, test, vi } from 'vitest'; import assert from 'node:assert/strict'; import fs from 'node:fs'; +import path from 'node:path'; import { connectionWorkspace, createTestClient, + grantedLease, + proxyWorkspace, recordedLeaseRelease, seedConnectionState, + type LeaseAllocateRequest, } from '../../__tests__/remote-connection.fixtures.ts'; import { materializeRemoteConnectionForCommand } from './connection-runtime.ts'; import { LEASE_ROLLBACK_RELEASE_FAILED_REASON } from './connection-lease-transaction.ts'; @@ -61,11 +65,15 @@ function materializeOnHost(options: { }); } +function writeProfile(remoteConfigPath: string) { + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); +} + test('a lease this command allocated is released when Metro preparation fails', async () => { const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( 'agent-device-connect-metro-fail-', ); - fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + writeProfile(remoteConfigPath); const release = recordedLeaseRelease(); const prepareFailure = metroPrepareFailure(); @@ -98,7 +106,7 @@ test('a lease this command renewed is kept when Metro preparation fails', async const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( 'agent-device-connect-metro-fail-reused-', ); - fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + writeProfile(remoteConfigPath); const release = recordedLeaseRelease(); seedConnectionState({ stateDir, @@ -140,11 +148,119 @@ test('a lease this command renewed is kept when Metro preparation fails', async fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('a lease the daemon says this request did not create is kept when Metro preparation fails', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-metro-fail-not-created-', + ); + writeProfile(remoteConfigPath); + const release = recordedLeaseRelease(); + const prepareFailure = metroPrepareFailure(); + + await assert.rejects( + async () => + await materializeOnHost({ + stateDir, + remoteConfigPath, + client: createTestClient({ + // Another session of the same tenant/run holds this lease; the daemon hands it to the + // repeat allocation instead of creating one, and only the answer says so. + allocate: async (request: LeaseAllocateRequest) => ({ + leaseId: 'lease-held-by-another-session', + tenantId: request.tenant, + runId: request.runId, + backend: request.leaseBackend ?? 'android-instance', + created: false, + }), + release: release.stub, + prepare: async () => { + throw prepareFailure; + }, + }), + }), + prepareFailure, + ); + + // The other session's record names that lease, so this failed command must leave it alive. + assert.equal(release.request, undefined); + + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('a lease allocated before a platform conflict is still released when acquisition fails', async () => { + const { tempRoot, stateDir, remoteConfigPath, flags } = proxyWorkspace( + 'agent-device-connect-platform-conflict-', + ); + const release = recordedLeaseRelease(); + + // The proxy grants an Android device for an ios-instance lease: allocation succeeded, the axis + // only disagrees once acquisition settles the platform afterwards, and the grant must not leak. + await assert.rejects( + async () => + await materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, stateDir, remoteConfig: remoteConfigPath }, + client: createTestClient({ + allocate: async (request) => + grantedLease(request, { deviceKey: 'android:mobile:emulator-5554' }), + release: release.stub, + }), + }), + (error: unknown) => + error instanceof AppError && error.details?.reason === 'CONNECTION_PLATFORM_CONFLICT', + ); + + assert.equal(release.request?.leaseId, 'lease-1'); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('a lease the session record already names survives an active-pointer write failure', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-pointer-fail-', + ); + writeProfile(remoteConfigPath); + const release = recordedLeaseRelease(); + + const originalRenameSync = fs.renameSync.bind(fs); + const writeFailure = new Error('pointer write failed'); + vi.spyOn(fs, 'renameSync').mockImplementation((oldPath, newPath) => { + if (String(newPath).endsWith(path.join('remote-connections', '.active-session.json'))) { + throw writeFailure; + } + return originalRenameSync(oldPath, newPath); + }); + + await assert.rejects( + async () => + await materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + // No Metro settings at all: the failure comes from the pointer write, not from Metro. + flags: { + ...metroFlags(stateDir, remoteConfigPath), + metroPublicBaseUrl: undefined, + metroProxyBaseUrl: undefined, + }, + client: createTestClient({ release: release.stub }), + }), + writeFailure, + ); + + // The durable session record names the lease: the pointer is a reader, not the commit point, + // and disconnect can still find the lease through the record. + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-android' })?.leaseId, 'lease-1'); + assert.equal(release.request, undefined); + + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('a rollback whose release failed reports it on the Metro failure instead of masking it', async () => { const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( 'agent-device-connect-metro-fail-release-', ); - fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + writeProfile(remoteConfigPath); const prepareFailure = metroPrepareFailure(); const releaseFailure = new AppError('COMMAND_FAILED', 'provider rejected the release'); diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index 6340182dba..a418c63a8d 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -11,7 +11,7 @@ import { type RemoteConnectionState, } from '@agent-device/remote/remote-connection-state'; import { AppError } from '@agent-device/kernel/errors'; -import type { BatchStep, Lease, LeaseDevice } from '@agent-device/contracts/client'; +import type { BatchStep, LeaseDevice } from '@agent-device/contracts/client'; import type { SessionRuntimeHints } from '@agent-device/kernel/contracts'; import type { CliFlags } from '@agent-device/contracts/command'; import type { AgentDeviceClient } from '@agent-device/command-surface'; @@ -22,8 +22,9 @@ import { import { profileToCliFlags } from '../remote-config-flags.ts'; import { acquireConnectionLease, - releaseLeaseThisCommandAllocated, + createLeaseLedger, resolveRequestedLeaseBackend, + type LeaseLedger, } from './connection-lease-transaction.ts'; import { prepareRuntimeForCommand, @@ -96,12 +97,13 @@ export async function materializeRemoteConnectionForCommand(options: { let nextState = state; let changed = !existingState; - // This call owns a lease it allocated from the moment the provider grants it until the persisted - // record names it, which is the one place ownership transfers. Every exit before that transfer - // ends the lease: a lease no record names is invisible to disconnect, and would hold the device - // until its TTL lapses. A lease this call renewed was already named by the stored record, so this - // path leaves it to disconnect in every case, including a failure. - let unrecordedLease: Lease | undefined; + // The ledger owns a lease this command created from the moment allocation returns it, before any + // later step can fail, and holds it until the durable session record names it — the one place + // ownership transfers. Every exit before that transfer ends the lease: a lease no record names is + // invisible to disconnect, and would hold the device until its TTL lapses. A lease this call + // renewed, or one the provider granted to another holder, is named elsewhere already, so this + // path leaves it to whoever does in every case, including a failure. + const leaseLedger = createLeaseLedger(); try { const acquired = await acquireConnectionLease({ command, @@ -109,10 +111,10 @@ export async function materializeRemoteConnectionForCommand(options: { state, nextFlags, positionals: options.positionals, + leaseLedger, }); nextState = acquired.state; changed = changed || acquired.changed; - unrecordedLease = acquired.unrecordedLease; const runtimePreparation = await prepareRuntimeForCommand({ command, @@ -132,16 +134,12 @@ export async function materializeRemoteConnectionForCommand(options: { changed, stateDir, state: nextState, + leaseLedger, preparedMetroCleanupOnFailure: runtimePreparation.preparedMetroCleanupOnFailure, metroCleanupToStop: runtimePreparation.metroCleanupToStop, }); } catch (error) { - throw await releaseLeaseThisCommandAllocated({ - client, - state: nextState, - acquiredLease: unrecordedLease, - failure: error, - }); + throw await leaseLedger.releaseOwnedOnFailure(error); } return { @@ -163,14 +161,22 @@ async function persistMaterializedConnection(options: { changed: boolean; stateDir: string; state: RemoteConnectionState; + leaseLedger: LeaseLedger; preparedMetroCleanupOnFailure?: RemoteConnectionState['metro']; metroCleanupToStop?: RemoteConnectionState['metro']; }): Promise { if (options.changed) { try { - writeRemoteConnectionState({ stateDir: options.stateDir, state: options.state }); + // The session record is the durable commit point: the moment it lands, it names the lease and + // the ledger stops owning it. The active-pointer update that follows is a convenience reader + // for the default session; its failure leaves the named lease owned by the record, not by us. + writeRemoteConnectionState({ + stateDir: options.stateDir, + state: options.state, + onSessionRecordCommitted: () => options.leaseLedger.transfer(options.state.leaseId), + }); } catch (error) { - // The caller's rollback ends the lease this write would have taken ownership of. + // The caller's rollback ends any lease the record never came to name. await stopMetroCleanup(options.preparedMetroCleanupOnFailure); throw error; } diff --git a/src/cli/commands/proxy-lease-device.ts b/src/cli/commands/proxy-lease-device.ts index 65be57dade..4a5635d7e4 100644 --- a/src/cli/commands/proxy-lease-device.ts +++ b/src/cli/commands/proxy-lease-device.ts @@ -6,13 +6,12 @@ import { } from '@agent-device/kernel/device'; import { buildConnectionDeviceKey, - mergeRemoteConnectionRequestMetadata, parseConnectionDeviceKey, resolveConnectionDeviceScope, type ConnectionDeviceScope, type RemoteConnectionState, } from '@agent-device/remote/remote-connection-state'; -import { AppError, errorMessage } from '@agent-device/kernel/errors'; +import { AppError } from '@agent-device/kernel/errors'; import { deviceSpecUnsupportedError, requireLeaseDeviceSpec, @@ -21,6 +20,7 @@ import type { Lease, LeaseDevice, LeaseDeviceSpec } from '@agent-device/contract import type { LeaseBackend } from '@agent-device/kernel/contracts'; import type { CliFlags } from '@agent-device/contracts/command'; import type { AgentDeviceClient } from '@agent-device/command-surface'; +import type { LeaseLedger } from './connection-lease-transaction.ts'; import { INTERNAL_COMMANDS, PUBLIC_COMMANDS } from '@agent-device/command-registry/catalog'; const proxyLeaseAllocatingCommands: ReadonlySet = new Set([ @@ -120,19 +120,22 @@ export async function bindLeaseNamedDevice(options: { flags: CliFlags; leaseBackend: LeaseBackend; materialized: MaterializedLease; + leaseLedger: LeaseLedger; allocateByDeviceKey(state: RemoteConnectionState, flags: CliFlags): Promise; }): Promise<{ state: RemoteConnectionState; materialized: MaterializedLease }> { - const { client, flags, materialized } = options; + const { client, flags, materialized, leaseLedger } = options; const { lease, acquired } = materialized; const { spec, keys: specKeys } = requireLeaseDeviceSpec(flags); const leased = parseConnectionDeviceKey(lease.deviceKey); if (acquired && specKeys.length > 0 && (!lease.device || !leased)) { - throw await releaseUnhonouredSpecLease(client, options.state, lease, specKeys); + throw await refuseUnhonouredSpecLease(leaseLedger, options.state, lease, specKeys); } if (leased) { const identityConflict = leasedIdentityConflict(options.state, flags, leased); if (identityConflict && acquired) { - await releaseLease(client, options.state, lease).catch(() => undefined); + // Ends through the ledger so this lease is never released twice, and a release that could + // not end it is still owned when the refusal below throws — the failure path reports it. + await leaseLedger.end(lease.leaseId); } if (identityConflict) throw identityConflict; if (!acquired) { @@ -157,7 +160,7 @@ export async function bindLeaseNamedDevice(options: { }; } if (!acquired) throw unboundLeaseError(options.state, lease); - await releaseLease(client, options.state, lease); + await leaseLedger.end(lease.leaseId); const resolved = await resolveProxyLeaseStateLocally(options); return { state: resolved.state, @@ -165,44 +168,30 @@ export async function bindLeaseNamedDevice(options: { }; } -async function releaseLease( - client: AgentDeviceClient, - state: RemoteConnectionState, - lease: Lease, -): Promise<{ released: boolean }> { - return await client.leases.release({ - tenant: state.tenant, - runId: state.runId, - leaseId: lease.leaseId, - leaseBackend: lease.backend, - ...mergeRemoteConnectionRequestMetadata(lease, state), - deviceKey: lease.deviceKey, - }); -} - /** - * Releases a new lease whose backend did not confirm the device spec, and returns the refusal. A - * release that fails is reported in the refusal, which still names the lease, rather than raised in - * its place: the lease then lapses after its inactivity window. + * Refuses a new lease whose backend did not confirm the device spec, ending it through the ledger. + * A release that could not end the lease is reported in the refusal, which still names the lease, + * rather than raised in its place, and the lease stays owned so the command's failure path reports + * it too. */ -async function releaseUnhonouredSpecLease( - client: AgentDeviceClient, +async function refuseUnhonouredSpecLease( + leaseLedger: LeaseLedger, state: RemoteConnectionState, lease: Lease, keys: readonly string[], ): Promise { - let release: { released: boolean; releaseError?: string }; - try { - release = { released: (await releaseLease(client, state, lease)).released }; - } catch (error) { - release = { released: false, releaseError: errorMessage(error) }; - } + const outcome = await leaseLedger.end(lease.leaseId); return deviceSpecUnsupportedError( keys, - release.released + outcome.released ? 'The proxy backend leased a device without confirming the requested device spec, so the lease was released.' : 'The proxy backend leased a device without confirming the requested device spec, and releasing the lease failed, so it may stay held until it expires.', - { session: state.session, leaseId: lease.leaseId, ...release }, + { + session: state.session, + leaseId: lease.leaseId, + released: outcome.released, + ...(outcome.error === undefined ? {} : { releaseError: outcome.error }), + }, ); } From cffa60dbada966f69d5cd42529e124e364445491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 12:00:18 +0100 Subject: [PATCH 05/11] fix(cli): stop the keyless-lease fallback when its first lease survives, and document created correctly The round-2 review found two gaps. - The keyless-lease fallback released the daemon-granted lease and ignored whether that release ended it. A throwing or refused release left the first lease owned by no durable record while the fallback allocated a replacement that became the connection. The fallback now requires a confirmed retirement; otherwise the command fails with PROXY_LEASE_RETIREMENT_FAILED, the ledger keeps the lease, and the failure path retries the release and reports it as details.leaseRollback beside the refusal. - The published JSDoc on Lease.created stated the mapping backwards; it now reads true = created by this allocation, false = an existing lease, absent = the daemon did not answer. website/docs/docs/client-api.md describes the field and the release guard: release only when created is true, and a missing answer is not permission to release. --- packages/contracts/src/client-lease.ts | 9 +- ...-connection-proxy-allocate-by-spec.test.ts | 83 +++++++++++++++++++ src/cli/commands/proxy-lease-device.ts | 29 ++++++- website/docs/docs/client-api.md | 2 +- 4 files changed, 117 insertions(+), 6 deletions(-) diff --git a/packages/contracts/src/client-lease.ts b/packages/contracts/src/client-lease.ts index 2f64daa625..97b820ffe8 100644 --- a/packages/contracts/src/client-lease.ts +++ b/packages/contracts/src/client-lease.ts @@ -15,11 +15,12 @@ export type Lease = { deviceKey?: string; clientId?: string; /** - * What an allocate answered about this request: `false` when the daemon created the lease for it, - * `true` when it handed back a lease that already existed — renewed, or already bound to the run. + * What an allocate answered about this request: `true` when the daemon created the lease for it, + * `false` when it handed back a lease that already existed — renewed, or already bound to the run. * The returned lease alone cannot tell the two apart, and a caller that ends a lease must end only - * the one its own request created. Only `leases.allocate` answers this; every other read, and any - * daemon older than this field, omits it. + * the one its own request created, so release on `created === true` and on nothing else. Only + * `leases.allocate` answers this; every other read, any daemon older than this field, and every + * lease you did not allocate yourself omit it, and an omitted answer is not permission to release. */ created?: boolean; /** diff --git a/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts b/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts index 48e49af9bf..bc9c2f5e6c 100644 --- a/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts +++ b/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts @@ -217,6 +217,89 @@ test('a daemon whose lease names no device gets that lease released and the devi fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('a keyless lease whose release throws stops the local fallback instead of abandoning it', async () => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-by-spec-old-release-'); + const requests: LeaseAllocateRequest[] = []; + const releases: LeaseReleaseRequest[] = []; + + await assert.rejects( + materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, device: 'iPhone 17' }, + client: createTestClient({ + listDevices: async () => [IPHONE_SIMULATOR], + allocate: async (request) => { + requests.push(request); + return grantedLease(request, { leaseId: `lease-${requests.length}` }); + }, + release: async (request) => { + releases.push(request); + throw new Error('provider unreachable'); + }, + }), + }), + (error: unknown) => { + const appError = error as AppError; + assert.equal(appError.code, 'COMMAND_FAILED'); + assert.equal(appError.details?.reason, 'PROXY_LEASE_RETIREMENT_FAILED'); + assert.equal(appError.details?.leaseId, 'lease-1'); + // The failed retirement rides on the refusal that names it, and the command's failure path + // retried the lease the ledger still owned. + assert.deepEqual(appError.details?.leaseRollback, { + leaseId: 'lease-1', + released: false, + reason: 'LEASE_RELEASE_FAILED', + error: 'provider unreachable', + }); + return true; + }, + ); + + // The fallback allocates no replacement for a lease it could not prove ended, so nothing beyond + // lease-1 exists, and the failed retry is reported rather than dropped. + assert.equal(requests.length, 1); + assert.deepEqual( + releases.map((request) => request.leaseId), + ['lease-1', 'lease-1'], + ); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('a keyless lease whose release is refused stops the local fallback instead of abandoning it', async () => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-by-spec-old-noop-'); + const requests: LeaseAllocateRequest[] = []; + + await assert.rejects( + materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, device: 'iPhone 17' }, + client: createTestClient({ + listDevices: async () => [IPHONE_SIMULATOR], + allocate: async (request) => { + requests.push(request); + return grantedLease(request, { leaseId: `lease-${requests.length}` }); + }, + release: async () => ({ released: false }), + }), + }), + (error: unknown) => { + const appError = error as AppError; + assert.equal(appError.details?.reason, 'PROXY_LEASE_RETIREMENT_FAILED'); + assert.equal(appError.details?.leaseId, 'lease-1'); + assert.equal(appError.details?.released, false); + return true; + }, + ); + + // A release that answers "did not end it" keeps the lease owned, so the failure path retries it. + assert.equal(requests.length, 1); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('a renewed lease that names no device is refused and left to its owner', async () => { const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-by-spec-keyless-', { leaseId: 'lease-1', diff --git a/src/cli/commands/proxy-lease-device.ts b/src/cli/commands/proxy-lease-device.ts index 4a5635d7e4..3852abdbc9 100644 --- a/src/cli/commands/proxy-lease-device.ts +++ b/src/cli/commands/proxy-lease-device.ts @@ -160,7 +160,7 @@ export async function bindLeaseNamedDevice(options: { }; } if (!acquired) throw unboundLeaseError(options.state, lease); - await leaseLedger.end(lease.leaseId); + await retireKeylessLeaseBeforeFallback(leaseLedger, options.state, lease); const resolved = await resolveProxyLeaseStateLocally(options); return { state: resolved.state, @@ -168,6 +168,33 @@ export async function bindLeaseNamedDevice(options: { }; } +/** + * Ends a new lease that names no device before the local-pick fallback allocates its replacement. + * The fallback would otherwise abandon this lease: only the replacement reaches the durable record, + * and a lease no record names is invisible to disconnect. An unconfirmed retirement therefore fails + * the command with typed release evidence; the ledger still owns the lease, so the command's failure + * path retries the release and reports it beside this refusal. + */ +async function retireKeylessLeaseBeforeFallback( + leaseLedger: LeaseLedger, + state: RemoteConnectionState, + lease: Lease, +): Promise { + const retired = await leaseLedger.end(lease.leaseId); + if (retired.released) return; + throw new AppError( + 'COMMAND_FAILED', + 'The proxy backend leased a device without naming it, and releasing that lease failed, so the local fallback stopped. The lease may stay held until it expires.', + { + session: state.session, + leaseId: lease.leaseId, + released: false, + reason: 'PROXY_LEASE_RETIREMENT_FAILED', + ...(retired.error === undefined ? {} : { releaseError: retired.error }), + }, + ); +} + /** * Refuses a new lease whose backend did not confirm the device spec, ending it through the ledger. * A release that could not end the lease is reported in the refusal, which still names the lease, diff --git a/website/docs/docs/client-api.md b/website/docs/docs/client-api.md index 8bce7e4e08..ad3e250d13 100644 --- a/website/docs/docs/client-api.md +++ b/website/docs/docs/client-api.md @@ -344,7 +344,7 @@ Domain client methods: - `client.sessions.list()`, `stateDir()`, `close()`, `saveScript()`, `artifacts()` - `client.apps.install()`, `reinstall()`, `uninstall()`, `installFromSource()`, `list()`, `open()`, `close()`, `push()`, `triggerEvent()` - `client.materializations.release()` -- `client.leases.allocate()`, `heartbeat()`, `release()`. Pass `retainOnClose: true` to `allocate()` when you release the lease yourself; session `close` then leaves it active until `release()`, expiry, or daemon shutdown. The returned lease has `retainOnClose: true` only when the daemon honored it. Keep heartbeating a retained lease between sessions: it expires after its `ttlMs` like any other lease. Pass `leaseId` (16 to 128 hex characters) to `allocate()` to name a new lease with an id the daemon has never issued, or to get back your live lease with that id when you retry with the same tenant, run, client, lease backend, and provider (and no device, or the leased one), then use the returned `leaseId`: a run that already holds a lease gets that one back, and an older daemon mints its own. `allocate()` with a `leaseProvider` the daemon has no runtime for fails with `UNSUPPORTED_OPERATION`, reason `provider-not-registered`, and `details.registeredProviders`; no lease is recorded. Leases with no provider, `proxy`, or `cloud` need no runtime. +- `client.leases.allocate()`, `heartbeat()`, `release()`. Pass `retainOnClose: true` to `allocate()` when you release the lease yourself; session `close` then leaves it active until `release()`, expiry, or daemon shutdown. The returned lease has `retainOnClose: true` only when the daemon honored it. Keep heartbeating a retained lease between sessions: it expires after its `ttlMs` like any other lease. Pass `leaseId` (16 to 128 hex characters) to `allocate()` to name a new lease with an id the daemon has never issued, or to get back your live lease with that id when you retry with the same tenant, run, client, lease backend, and provider (and no device, or the leased one), then use the returned `leaseId`: a run that already holds a lease gets that one back, and an older daemon mints its own. `allocate()` also tells you which of those you got, on the returned lease's `created`: `true` when your request created that lease, `false` when it was already there, and no value at all when the daemon does not answer, as with `heartbeat()`. Release a lease only when `created` is `true`. A lease you did not create belongs to whoever did, and a missing answer is not permission to release one. `allocate()` with a `leaseProvider` the daemon has no runtime for fails with `UNSUPPORTED_OPERATION`, reason `provider-not-registered`, and `details.registeredProviders`; no lease is recorded. Leases with no provider, `proxy`, or `cloud` need no runtime. - `client.metro.prepare()`, `reload()` - `client.capture.snapshot()`, `screenshot()`, `diff()` - `client.interactions.gesture()`, `click()`, `press()`, `longPress()`, `swipe()`, `pan()`, `drag()`, `fling()`, `swipeGesture()`, `focus()`, `type()`, `fill()`, `scroll()`, `pinch()`, `rotateGesture()`, `transformGesture()`, `tapGesture()`, `get()`, `is()`, `find()` From 4472e0782c4fdc33b7484b40cf00f74efff5a6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 22:12:39 +0100 Subject: [PATCH 06/11] fix: preserve older daemon allocation binding and simplify lease rollback --- packages/client/src/lease-client.ts | 2 +- .../sessions/handlers/__tests__/lease.test.ts | 10 +- ...-connection-proxy-allocate-by-spec.test.ts | 84 +++++++---- .../__tests__/proxy-lease-device-spec.test.ts | 33 +++++ .../commands/connection-lease-transaction.ts | 137 +++++------------- .../connection-runtime-lease-rollback.test.ts | 67 +++++++++ src/cli/commands/proxy-lease-device.ts | 47 ++---- website/docs/docs/client-api.md | 2 +- website/docs/docs/remote-proxy.md | 2 +- 9 files changed, 213 insertions(+), 171 deletions(-) diff --git a/packages/client/src/lease-client.ts b/packages/client/src/lease-client.ts index 28bd5d91b8..a594723cd6 100644 --- a/packages/client/src/lease-client.ts +++ b/packages/client/src/lease-client.ts @@ -96,7 +96,7 @@ function normalizeLease(data: Record): Lease { deviceKey: readOptionalString(rawLease, 'deviceKey'), // Only an allocate answers `reused`; every other read, and any daemon older than the field, // omits it, and the caller keeps `created` unset rather than guessing who made the lease. - ...(data.reused === true ? { created: false } : data.reused === false ? { created: true } : {}), + ...(typeof data.reused === 'boolean' ? { created: !data.reused } : {}), ...withLeaseDevice(readLeaseDevice(rawLease.device)), ...(rawLease.retainOnClose === true ? { retainOnClose: true } : {}), createdAt: typeof rawLease.createdAt === 'number' ? rawLease.createdAt : undefined, diff --git a/packages/daemon/src/sessions/handlers/__tests__/lease.test.ts b/packages/daemon/src/sessions/handlers/__tests__/lease.test.ts index af30bccd9f..4f32d87814 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/lease.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/lease.test.ts @@ -83,7 +83,7 @@ test('lease_allocate answers with the lease id the request named', async () => { test('lease_allocate answers whether its request created the lease or got back an existing one', async () => { const registry = new LeaseRegistry(); const leaseId = 'abcdef0123456789abcdef0123456789'; - const allocate = () => + const allocate = (requestedLeaseId?: string) => handleLeaseCommands({ host: createTestHostEnvironment(), req: { @@ -91,14 +91,18 @@ test('lease_allocate answers whether its request created the lease or got back a session: 'default', command: 'lease_allocate', positionals: [], - meta: { tenantId: 'tenant-a', runId: 'run-1', leaseId }, + meta: { + tenantId: 'tenant-a', + runId: 'run-1', + ...(requestedLeaseId ? { leaseId: requestedLeaseId } : {}), + }, }, sessionName: 'default', sessionStore: makeSessionStore('agent-device-allocate-reused-'), leaseRegistry: registry, }); - const created = await allocate(); + const created = await allocate(leaseId); assert.ok(created?.ok); assert.equal(created.data?.reused, false); // A repeat for the same run binding gets that lease back, and the answer says this request did diff --git a/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts b/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts index bc9c2f5e6c..80669f09c4 100644 --- a/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts +++ b/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts @@ -217,6 +217,38 @@ test('a daemon whose lease names no device gets that lease released and the devi fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('a pre-pick daemon omitting created falls back locally without releasing either allocation', async () => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-pre-pick-'); + const requests: LeaseAllocateRequest[] = []; + const releases: LeaseReleaseRequest[] = []; + const materialized = await materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, device: 'iPhone 17' }, + client: createTestClient({ + listDevices: async () => [IPHONE_SIMULATOR], + allocate: async (request) => { + requests.push(request); + const { created: _created, ...lease } = grantedLease(request, { + leaseId: `lease-${requests.length}`, + }); + return lease; + }, + release: async (request) => { + releases.push(request); + return { released: true }; + }, + }), + }); + assert.equal(requests.length, 2); + assert.equal(requests[0]?.deviceKey, undefined); + assert.equal(requests[1]?.deviceKey, 'ios:mobile:SIM-LOCAL'); + assert.equal(materialized.flags.udid, 'SIM-LOCAL'); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, 'lease-2'); + assert.deepEqual(releases, []); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('a keyless lease whose release throws stops the local fallback instead of abandoning it', async () => { const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-by-spec-old-release-'); const requests: LeaseAllocateRequest[] = []; @@ -242,8 +274,7 @@ test('a keyless lease whose release throws stops the local fallback instead of a (error: unknown) => { const appError = error as AppError; assert.equal(appError.code, 'COMMAND_FAILED'); - assert.equal(appError.details?.reason, 'PROXY_LEASE_RETIREMENT_FAILED'); - assert.equal(appError.details?.leaseId, 'lease-1'); + assert.equal(appError.message, 'provider unreachable'); // The failed retirement rides on the refusal that names it, and the command's failure path // retried the lease the ledger still owned. assert.deepEqual(appError.details?.leaseRollback, { @@ -267,36 +298,33 @@ test('a keyless lease whose release throws stops the local fallback instead of a fs.rmSync(tempRoot, { recursive: true, force: true }); }); -test('a keyless lease whose release is refused stops the local fallback instead of abandoning it', async () => { +test('an already absent keyless lease permits fallback and is released only once', async () => { const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-by-spec-old-noop-'); const requests: LeaseAllocateRequest[] = []; - - await assert.rejects( - materializeRemoteConnectionForCommand({ - host: createTestHostEnvironment(), - command: 'open', - flags: { ...flags, device: 'iPhone 17' }, - client: createTestClient({ - listDevices: async () => [IPHONE_SIMULATOR], - allocate: async (request) => { - requests.push(request); - return grantedLease(request, { leaseId: `lease-${requests.length}` }); - }, - release: async () => ({ released: false }), - }), + const releases: LeaseReleaseRequest[] = []; + const materialized = await materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, device: 'iPhone 17' }, + client: createTestClient({ + listDevices: async () => [IPHONE_SIMULATOR], + allocate: async (request) => { + requests.push(request); + return grantedLease(request, { leaseId: `lease-${requests.length}` }); + }, + release: async (request) => { + releases.push(request); + return { released: false }; + }, }), - (error: unknown) => { - const appError = error as AppError; - assert.equal(appError.details?.reason, 'PROXY_LEASE_RETIREMENT_FAILED'); - assert.equal(appError.details?.leaseId, 'lease-1'); - assert.equal(appError.details?.released, false); - return true; - }, + }); + assert.equal(requests.length, 2); + assert.deepEqual( + releases.map((request) => request.leaseId), + ['lease-1'], ); - - // A release that answers "did not end it" keeps the lease owned, so the failure path retries it. - assert.equal(requests.length, 1); - assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + assert.equal(materialized.flags.udid, 'SIM-LOCAL'); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, 'lease-2'); fs.rmSync(tempRoot, { recursive: true, force: true }); }); diff --git a/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts b/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts index cfccbb22b6..e087e29346 100644 --- a/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts +++ b/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts @@ -100,6 +100,39 @@ test('proxy open by device spec sends the spec on allocate without listing devic fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('a by-spec grant omitting created binds and records the granted spec without releasing it', async () => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-spec-old-daemon-'); + const releases: LeaseReleaseRequest[] = []; + const materialized = await materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, osVersion: '>=26' }, + client: createTestClient({ + listDevices: unexpectedCommandCall, + allocate: async (request) => { + const { created: _created, ...lease } = grantedLease(request, { + deviceKey: REMOTE_DEVICE_KEY, + device: GRANTED_DEVICE, + }); + return lease; + }, + release: async (request) => { + releases.push(request); + return { released: true }; + }, + }), + }); + assert.equal(materialized.flags.udid, 'SIM-REMOTE'); + assert.deepEqual(materialized.leaseDevice, GRANTED_DEVICE); + assert.deepEqual(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseDeviceSpec, { + leaseId: 'lease-1', + spec: { osVersion: '>=26' }, + device: GRANTED_DEVICE, + }); + assert.deepEqual(releases, []); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('proxy open by device spec without a platform or lease backend fails before allocating', async () => { const { tempRoot, flags } = proxyWorkspace('agent-device-proxy-device-spec-no-platform-'); const { platform: _platform, ...flagsWithoutPlatform } = flags; diff --git a/src/cli/commands/connection-lease-transaction.ts b/src/cli/commands/connection-lease-transaction.ts index 528b88f7f2..8383051c42 100644 --- a/src/cli/commands/connection-lease-transaction.ts +++ b/src/cli/commands/connection-lease-transaction.ts @@ -23,103 +23,54 @@ import { } from './connection-lease-policy.ts'; import { bindLeaseNamedDevice } from './proxy-lease-device.ts'; -/** Whether a release attempt ended the lease, and why not when it did not. */ -export type LeaseReleaseOutcome = { released: boolean; error?: string }; - type LeaseRelease = () => Promise<{ released: boolean }>; -/** - * The lease one command allocated and no durable record has taken over yet — the ledger the - * materialization consults before ending anything. A command owns a lease from the moment allocate - * returns it as created by this command, and ownership ends at the first durable write that names - * it, or when a deliberate release ends it. A lease this command renewed, or that the daemon says it - * did not create, was named elsewhere first: this path never ends it, because the record or the - * other run that names it owns it through any failure here. - */ +/** One newly created lease awaits release or transfer to a durable session record. */ export type LeaseLedger = { - /** Takes the lease this command created, with the release that ends it. */ track(leaseId: string, release: LeaseRelease): void; - /** - * Ends an owned lease through its recorded release. The lease stays owned when the release did - * not end it, so the failure path reports and retries it; a release already in flight is never - * attempted twice. - */ - end(leaseId: string): Promise; - /** A durable record names the lease now: this command no longer owns it. */ + /** A resolved release ends ownership, including when the daemon reports no such lease. */ + end(leaseId: string): Promise<{ released: boolean }>; transfer(leaseId: string | undefined): void; - /** Ends every lease still owned and reports the ones that stay held beside `failure`. */ releaseOwnedOnFailure(failure: unknown): Promise; }; export function createLeaseLedger(): LeaseLedger { - const owned = new Map(); + let owned: { leaseId: string; release: LeaseRelease } | undefined; - async function attempt( - leaseId: string, - entry: { release: LeaseRelease }, - ): Promise { - try { - const outcome = await entry.release(); - if (outcome.released) owned.delete(leaseId); - return outcome.released - ? { released: true } - : { released: false, error: 'the release did not end the lease' }; - } catch (error) { - return { released: false, error: errorMessage(error) }; - } + async function end(leaseId: string): Promise<{ released: boolean }> { + if (owned?.leaseId !== leaseId) return { released: false }; + const outcome = await owned.release(); + owned = undefined; + return outcome; } return { track(leaseId, release) { - owned.set(leaseId, { release, inFlight: false }); - }, - async end(leaseId) { - const entry = owned.get(leaseId); - if (entry === undefined) - return { released: false, error: 'this command never owned the lease' }; - if (entry.inFlight) - return { released: false, error: 'a release for this lease is already in flight' }; - entry.inFlight = true; - try { - return await attempt(leaseId, entry); - } finally { - entry.inFlight = false; - } + owned = { leaseId, release }; }, + end, transfer(leaseId) { - if (leaseId !== undefined) owned.delete(leaseId); + if (owned?.leaseId === leaseId) owned = undefined; }, async releaseOwnedOnFailure(failure) { - const unended: { leaseId: string; error: string }[] = []; - for (const [leaseId, entry] of Array.from(owned)) { - // An outcome this command is still waiting on is reported, not retried: the lease may or - // may not have ended, and a second release beside the first is exactly what the ledger - // exists to make impossible. - const outcome = entry.inFlight - ? { released: false, error: 'a release for this lease was still in flight' } - : await attempt(leaseId, entry); - if (!outcome.released) - unended.push({ leaseId, error: outcome.error ?? 'the release was refused' }); + if (!owned) return failure; + const { leaseId } = owned; + try { + await end(leaseId); + return failure; + } catch (error) { + const primary = asAppError(failure, 'COMMAND_FAILED'); + primary.details = { + ...primary.details, + leaseRollback: { + leaseId, + released: false, + reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, + error: errorMessage(error), + }, + }; + return primary; } - if (unended.length === 0) return failure; - const first = unended[0]!; - // The command's own failure stays the thrown error (ADR 0010, as amended by ADR 0019), and a - // release that could not end the lease rides beside it as typed evidence naming the lease that - // stays held until its TTL lapses. - const primary = asAppError(failure, 'COMMAND_FAILED'); - primary.details = { - ...primary.details, - leaseRollback: { - leaseId: first.leaseId, - released: false, - reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, - error: first.error, - ...(unended.length > 1 - ? { additionalLeaseIds: unended.slice(1).map((entry) => entry.leaseId) } - : {}), - }, - }; - return primary; }, }; } @@ -227,28 +178,14 @@ async function materializeLeaseForCommand(options: { preliminaryLeaseBackend ?? requireRequestedLeaseBackend(nextFlags, command); 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. + const platform = carriedConnectionPlatform( + state.session, + { ...nextState, leaseBackend }, + nextFlags, + ); assertRequestedConnectionTarget(state, nextFlags); - nextState = { ...nextState, platform: platform.platform }; - nextFlags.platform = platform.platform; + nextState = { ...nextState, platform }; + nextFlags.platform = platform; let materializedLease = await allocateOrReuseLease( client, nextState, @@ -447,7 +384,7 @@ async function allocateOrReuseLease( // Ownership starts here, before any caller of this function can fail on this lease. leaseLedger.track(lease.leaseId, leaseAllocationRelease(client, state, leaseBackend, lease)); } - return { lease, acquired: created }; + return { lease, acquired: true }; } /** diff --git a/src/cli/commands/connection-runtime-lease-rollback.test.ts b/src/cli/commands/connection-runtime-lease-rollback.test.ts index 903c84c228..b1b0811f97 100644 --- a/src/cli/commands/connection-runtime-lease-rollback.test.ts +++ b/src/cli/commands/connection-runtime-lease-rollback.test.ts @@ -186,6 +186,73 @@ test('a lease the daemon says this request did not create is kept when Metro pre fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('an allocation omitting created is kept when Metro preparation fails', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-old-daemon-', + ); + writeProfile(remoteConfigPath); + const releases: string[] = []; + const prepareFailure = metroPrepareFailure(); + await assert.rejects( + materializeOnHost({ + stateDir, + remoteConfigPath, + client: createTestClient({ + allocate: async (request) => { + const { created: _created, ...lease } = grantedLease(request); + return lease; + }, + release: async (request) => { + releases.push(request.leaseId); + return { released: true }; + }, + prepare: async () => { + throw prepareFailure; + }, + }), + }), + (error) => { + assert.equal(error, prepareFailure); + assert.equal(prepareFailure.details?.leaseRollback, undefined); + return true; + }, + ); + assert.deepEqual(releases, []); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-android' }), null); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('an already absent lease adds no rollback failure to the Metro error', async () => { + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-connect-expired-', + ); + writeProfile(remoteConfigPath); + const releases: string[] = []; + const prepareFailure = metroPrepareFailure(); + await assert.rejects( + materializeOnHost({ + stateDir, + remoteConfigPath, + client: createTestClient({ + release: async (request) => { + releases.push(request.leaseId); + return { released: false }; + }, + prepare: async () => { + throw prepareFailure; + }, + }), + }), + (error) => { + assert.equal(error, prepareFailure); + assert.equal(prepareFailure.details?.leaseRollback, undefined); + return true; + }, + ); + assert.deepEqual(releases, ['lease-1']); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('a lease allocated before a platform conflict is still released when acquisition fails', async () => { const { tempRoot, stateDir, remoteConfigPath, flags } = proxyWorkspace( 'agent-device-connect-platform-conflict-', diff --git a/src/cli/commands/proxy-lease-device.ts b/src/cli/commands/proxy-lease-device.ts index 3852abdbc9..e26158ea54 100644 --- a/src/cli/commands/proxy-lease-device.ts +++ b/src/cli/commands/proxy-lease-device.ts @@ -11,7 +11,7 @@ import { type ConnectionDeviceScope, type RemoteConnectionState, } from '@agent-device/remote/remote-connection-state'; -import { AppError } from '@agent-device/kernel/errors'; +import { AppError, errorMessage } from '@agent-device/kernel/errors'; import { deviceSpecUnsupportedError, requireLeaseDeviceSpec, @@ -104,8 +104,8 @@ async function resolveProxyLeaseStateLocally(options: { /** * Takes the device from a lease allocated by spec: the one its `deviceKey` names, whether the daemon * just picked it or the lease is renewed. A daemon that predates picking leaves the key of a new - * lease unset, so that lease is released and the device is picked here and allocated by its key, as - * before. Every lease this client allocates therefore names its device; a renewed lease that names + * lease unset, so an owned lease is released before the device is picked here and allocated by its + * key. Every lease this client allocates therefore names its device; a renewed lease that names * none was supplied from outside, so it is refused and never released. * * A request that names a device spec has no such fallback: a new lease that does not echo the @@ -132,11 +132,6 @@ export async function bindLeaseNamedDevice(options: { } if (leased) { const identityConflict = leasedIdentityConflict(options.state, flags, leased); - if (identityConflict && acquired) { - // Ends through the ledger so this lease is never released twice, and a release that could - // not end it is still owned when the refusal below throws — the failure path reports it. - await leaseLedger.end(lease.leaseId); - } if (identityConflict) throw identityConflict; if (!acquired) { assertLeaseSpecMatchesRequest(options.state, lease, spec, specKeys, leased); @@ -160,7 +155,7 @@ export async function bindLeaseNamedDevice(options: { }; } if (!acquired) throw unboundLeaseError(options.state, lease); - await retireKeylessLeaseBeforeFallback(leaseLedger, options.state, lease); + await leaseLedger.end(lease.leaseId); const resolved = await resolveProxyLeaseStateLocally(options); return { state: resolved.state, @@ -168,33 +163,6 @@ export async function bindLeaseNamedDevice(options: { }; } -/** - * Ends a new lease that names no device before the local-pick fallback allocates its replacement. - * The fallback would otherwise abandon this lease: only the replacement reaches the durable record, - * and a lease no record names is invisible to disconnect. An unconfirmed retirement therefore fails - * the command with typed release evidence; the ledger still owns the lease, so the command's failure - * path retries the release and reports it beside this refusal. - */ -async function retireKeylessLeaseBeforeFallback( - leaseLedger: LeaseLedger, - state: RemoteConnectionState, - lease: Lease, -): Promise { - const retired = await leaseLedger.end(lease.leaseId); - if (retired.released) return; - throw new AppError( - 'COMMAND_FAILED', - 'The proxy backend leased a device without naming it, and releasing that lease failed, so the local fallback stopped. The lease may stay held until it expires.', - { - session: state.session, - leaseId: lease.leaseId, - released: false, - reason: 'PROXY_LEASE_RETIREMENT_FAILED', - ...(retired.error === undefined ? {} : { releaseError: retired.error }), - }, - ); -} - /** * Refuses a new lease whose backend did not confirm the device spec, ending it through the ledger. * A release that could not end the lease is reported in the refusal, which still names the lease, @@ -207,7 +175,12 @@ async function refuseUnhonouredSpecLease( lease: Lease, keys: readonly string[], ): Promise { - const outcome = await leaseLedger.end(lease.leaseId); + let outcome: { released: boolean; error?: string }; + try { + outcome = await leaseLedger.end(lease.leaseId); + } catch (error) { + outcome = { released: false, error: errorMessage(error) }; + } return deviceSpecUnsupportedError( keys, outcome.released diff --git a/website/docs/docs/client-api.md b/website/docs/docs/client-api.md index ad3e250d13..3b8095ccf7 100644 --- a/website/docs/docs/client-api.md +++ b/website/docs/docs/client-api.md @@ -344,7 +344,7 @@ Domain client methods: - `client.sessions.list()`, `stateDir()`, `close()`, `saveScript()`, `artifacts()` - `client.apps.install()`, `reinstall()`, `uninstall()`, `installFromSource()`, `list()`, `open()`, `close()`, `push()`, `triggerEvent()` - `client.materializations.release()` -- `client.leases.allocate()`, `heartbeat()`, `release()`. Pass `retainOnClose: true` to `allocate()` when you release the lease yourself; session `close` then leaves it active until `release()`, expiry, or daemon shutdown. The returned lease has `retainOnClose: true` only when the daemon honored it. Keep heartbeating a retained lease between sessions: it expires after its `ttlMs` like any other lease. Pass `leaseId` (16 to 128 hex characters) to `allocate()` to name a new lease with an id the daemon has never issued, or to get back your live lease with that id when you retry with the same tenant, run, client, lease backend, and provider (and no device, or the leased one), then use the returned `leaseId`: a run that already holds a lease gets that one back, and an older daemon mints its own. `allocate()` also tells you which of those you got, on the returned lease's `created`: `true` when your request created that lease, `false` when it was already there, and no value at all when the daemon does not answer, as with `heartbeat()`. Release a lease only when `created` is `true`. A lease you did not create belongs to whoever did, and a missing answer is not permission to release one. `allocate()` with a `leaseProvider` the daemon has no runtime for fails with `UNSUPPORTED_OPERATION`, reason `provider-not-registered`, and `details.registeredProviders`; no lease is recorded. Leases with no provider, `proxy`, or `cloud` need no runtime. +- `client.leases.allocate()`, `heartbeat()`, `release()`. Pass `retainOnClose: true` to `allocate()` when you release the lease yourself; session `close` then leaves it active until `release()`, expiry, or daemon shutdown. The returned lease has `retainOnClose: true` only when the daemon honored it. Keep heartbeating a retained lease between sessions: it expires after its `ttlMs` like any other lease. Pass `leaseId` (16 to 128 hex characters) to `allocate()` to name a new lease with an id the daemon has never issued, or to get back your live lease with that id when you retry with the same tenant, run, client, lease backend, and provider (and no device, or the leased one), then use the returned `leaseId`: a run that already holds a lease gets that one back, and an older daemon mints its own. `allocate()` also tells you which of those you got, on the returned lease's `created`: `true` when your request created that lease, `false` when it was already there, and omitted on `heartbeat()` or when an older daemon does not support ownership evidence. Release a lease only when `created` is `true`. A lease you did not create belongs to whoever did, and an omitted `created` is not permission to release one. Against an older daemon, the CLI can still bind the allocated device, but cannot roll back an unrecorded lease after preparation fails; that lease remains until expiry. `allocate()` with a `leaseProvider` the daemon has no runtime for fails with `UNSUPPORTED_OPERATION`, reason `provider-not-registered`, and `details.registeredProviders`; no lease is recorded. Leases with no provider, `proxy`, or `cloud` need no runtime. - `client.metro.prepare()`, `reload()` - `client.capture.snapshot()`, `screenshot()`, `diff()` - `client.interactions.gesture()`, `click()`, `press()`, `longPress()`, `swipe()`, `pan()`, `drag()`, `fling()`, `swipeGesture()`, `focus()`, `type()`, `fill()`, `scroll()`, `pinch()`, `rotateGesture()`, `transformGesture()`, `tapGesture()`, `get()`, `is()`, `find()` diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index 41916611af..6b339e644f 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -75,7 +75,7 @@ With `--physical`, the service leases one of the real phones enrolled with it in A service that honours the spec returns the granted device on the lease as `lease.device`: `model`, `osVersion` (the exact OS granted), `imageTag` when one applies, and `physical`. `open --json` prints it under `lease.device`. -The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI releases that lease and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. `details.released` reports whether the release succeeded. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. +The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI releases that lease and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. `details.released` reports whether the release succeeded. If rollback also fails, `details.leaseRollback` names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits allocation ownership evidence leaves an unrecorded lease until expiry when preparation fails. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. ## Download captures From c36e0f1e21ca5e66eac07ad8e1400fd73cc7178c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 22:40:35 +0100 Subject: [PATCH 07/11] test: assert fallback lease binds the selected device --- .../remote-connection-proxy-allocate-by-spec.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts b/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts index 80669f09c4..f4fe9b5696 100644 --- a/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts +++ b/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts @@ -244,7 +244,10 @@ test('a pre-pick daemon omitting created falls back locally without releasing ei assert.equal(requests[0]?.deviceKey, undefined); assert.equal(requests[1]?.deviceKey, 'ios:mobile:SIM-LOCAL'); assert.equal(materialized.flags.udid, 'SIM-LOCAL'); - assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, 'lease-2'); + assert.equal(materialized.connection?.deviceKey, 'ios:mobile:SIM-LOCAL'); + const state = readRemoteConnectionState({ stateDir, session: 'adc-proxy' }); + assert.equal(state?.leaseId, 'lease-2'); + assert.equal(state?.deviceKey, 'ios:mobile:SIM-LOCAL'); assert.deepEqual(releases, []); fs.rmSync(tempRoot, { recursive: true, force: true }); }); From 8ee729017c8863d052e3771c2a48ac7a956c16f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sun, 1 Nov 2026 23:31:24 +0100 Subject: [PATCH 08/11] fix: report spec lease rollback failures in one typed shape --- .../__tests__/proxy-lease-device-spec.test.ts | 67 ++++++++++++++++--- .../commands/connection-lease-transaction.ts | 6 +- .../connection-runtime-lease-rollback.test.ts | 3 +- src/cli/commands/proxy-lease-device.ts | 23 +++---- website/docs/docs/remote-proxy.md | 2 +- 5 files changed, 72 insertions(+), 29 deletions(-) diff --git a/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts b/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts index e087e29346..36401b92ba 100644 --- a/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts +++ b/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts @@ -185,11 +185,11 @@ test('a lease returned without lease.device for a spec request is released and t fs.rmSync(tempRoot, { recursive: true, force: true }); }); -test('a failed release of an unconfirmed spec lease still fails the command with device-spec-unsupported', async () => { +test('an unconfirmed spec lease gets exactly one rollback retry and one typed failure shape', async () => { const { tempRoot, stateDir, flags } = proxyWorkspace( 'agent-device-proxy-device-spec-release-fails-', ); - + const releases: LeaseReleaseRequest[] = []; await assert.rejects( materializeRemoteConnectionForCommand({ host: createTestHostEnvironment(), @@ -198,17 +198,66 @@ test('a failed release of an unconfirmed spec lease still fails the command with client: createTestClient({ listDevices: unexpectedCommandCall, allocate: async (request) => grantedLease(request, { deviceKey: REMOTE_DEVICE_KEY }), - release: async () => { + release: async (request) => { + releases.push(request); throw new AppError('COMMAND_FAILED', 'backend unreachable'); }, }), }), - (error: AppError) => - error.code === 'UNSUPPORTED_OPERATION' && - error.details?.reason === 'device-spec-unsupported' && - error.details?.released === false && - error.details?.releaseError === 'backend unreachable' && - error.details?.leaseId === 'lease-1', + (error: AppError) => { + assert.equal(error.code, 'UNSUPPORTED_OPERATION'); + assert.equal(error.details?.reason, 'device-spec-unsupported'); + assert.equal(error.details?.leaseId, 'lease-1'); + assert.deepEqual(error.details?.leaseRollback, { + leaseId: 'lease-1', + released: false, + reason: 'LEASE_RELEASE_FAILED', + error: 'backend unreachable', + }); + assert.equal(error.details?.released, undefined); + assert.equal(error.details?.releaseError, undefined); + return true; + }, + ); + assert.deepEqual( + releases.map((request) => request.leaseId), + ['lease-1', 'lease-1'], + ); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + +test('a successful release retry preserves the spec refusal without stale rollback failure evidence', async () => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-spec-release-recovers-'); + const releases: LeaseReleaseRequest[] = []; + await assert.rejects( + materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, physical: true }, + client: createTestClient({ + listDevices: unexpectedCommandCall, + allocate: async (request) => grantedLease(request, { deviceKey: REMOTE_DEVICE_KEY }), + release: async (request) => { + releases.push(request); + if (releases.length === 1) + throw new AppError('COMMAND_FAILED', 'temporary release failure'); + return { released: true }; + }, + }), + }), + (error: AppError) => { + assert.equal(error.code, 'UNSUPPORTED_OPERATION'); + assert.equal(error.details?.reason, 'device-spec-unsupported'); + assert.equal(error.details?.leaseRollback, undefined); + assert.equal(error.details?.releaseError, undefined); + assert.equal(error.details?.released, undefined); + return true; + }, + ); + assert.deepEqual( + releases.map((request) => request.leaseId), + ['lease-1', 'lease-1'], ); assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); fs.rmSync(tempRoot, { recursive: true, force: true }); diff --git a/src/cli/commands/connection-lease-transaction.ts b/src/cli/commands/connection-lease-transaction.ts index 8383051c42..ac2a43520d 100644 --- a/src/cli/commands/connection-lease-transaction.ts +++ b/src/cli/commands/connection-lease-transaction.ts @@ -25,8 +25,11 @@ import { bindLeaseNamedDevice } from './proxy-lease-device.ts'; type LeaseRelease = () => Promise<{ released: boolean }>; +const LEASE_ROLLBACK_RELEASE_FAILED_REASON = 'LEASE_RELEASE_FAILED'; + /** One newly created lease awaits release or transfer to a durable session record. */ export type LeaseLedger = { + /** Release or transfer the previous lease before tracking another allocation. */ track(leaseId: string, release: LeaseRelease): void; /** A resolved release ends ownership, including when the daemon reports no such lease. */ end(leaseId: string): Promise<{ released: boolean }>; @@ -75,9 +78,6 @@ export function createLeaseLedger(): LeaseLedger { }; } -/** How a rollback reports a lease its release could not end, beside the command's own failure. */ -export const LEASE_ROLLBACK_RELEASE_FAILED_REASON = 'LEASE_RELEASE_FAILED'; - /** * Brings the connection's lease up to date for this command: renews the one the record names or * allocates the one it does not, then settles the platform axis that decides which device that lease diff --git a/src/cli/commands/connection-runtime-lease-rollback.test.ts b/src/cli/commands/connection-runtime-lease-rollback.test.ts index b1b0811f97..6e1d14a1f0 100644 --- a/src/cli/commands/connection-runtime-lease-rollback.test.ts +++ b/src/cli/commands/connection-runtime-lease-rollback.test.ts @@ -13,7 +13,6 @@ import { type LeaseAllocateRequest, } from '../../__tests__/remote-connection.fixtures.ts'; import { materializeRemoteConnectionForCommand } from './connection-runtime.ts'; -import { LEASE_ROLLBACK_RELEASE_FAILED_REASON } from './connection-lease-transaction.ts'; import { readRemoteConnectionState } from '@agent-device/remote/remote-connection-state'; import { AppError } from '@agent-device/kernel/errors'; @@ -360,7 +359,7 @@ test('a rollback whose release failed reports it on the Metro failure instead of assert.deepEqual(thrown.details?.leaseRollback, { leaseId: 'lease-1', released: false, - reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, + reason: 'LEASE_RELEASE_FAILED', error: 'provider rejected the release', }); diff --git a/src/cli/commands/proxy-lease-device.ts b/src/cli/commands/proxy-lease-device.ts index e26158ea54..075508291f 100644 --- a/src/cli/commands/proxy-lease-device.ts +++ b/src/cli/commands/proxy-lease-device.ts @@ -11,7 +11,7 @@ import { type ConnectionDeviceScope, type RemoteConnectionState, } from '@agent-device/remote/remote-connection-state'; -import { AppError, errorMessage } from '@agent-device/kernel/errors'; +import { AppError } from '@agent-device/kernel/errors'; import { deviceSpecUnsupportedError, requireLeaseDeviceSpec, @@ -164,10 +164,8 @@ export async function bindLeaseNamedDevice(options: { } /** - * Refuses a new lease whose backend did not confirm the device spec, ending it through the ledger. - * A release that could not end the lease is reported in the refusal, which still names the lease, - * rather than raised in its place, and the lease stays owned so the command's failure path reports - * it too. + * Refuses an unconfirmed spec and attempts release. A thrown release leaves ownership with the + * caller for one rollback retry; only that retry reports failure, so recovery leaves no stale evidence. */ async function refuseUnhonouredSpecLease( leaseLedger: LeaseLedger, @@ -175,22 +173,19 @@ async function refuseUnhonouredSpecLease( lease: Lease, keys: readonly string[], ): Promise { - let outcome: { released: boolean; error?: string }; + let released: boolean | undefined; try { - outcome = await leaseLedger.end(lease.leaseId); - } catch (error) { - outcome = { released: false, error: errorMessage(error) }; + ({ released } = await leaseLedger.end(lease.leaseId)); + } catch { + // The caller retries the owned lease and attaches any failure as details.leaseRollback. } return deviceSpecUnsupportedError( keys, - outcome.released - ? 'The proxy backend leased a device without confirming the requested device spec, so the lease was released.' - : 'The proxy backend leased a device without confirming the requested device spec, and releasing the lease failed, so it may stay held until it expires.', + 'The proxy backend leased a device without confirming the requested device spec.', { session: state.session, leaseId: lease.leaseId, - released: outcome.released, - ...(outcome.error === undefined ? {} : { releaseError: outcome.error }), + ...(released === undefined ? {} : { released }), }, ); } diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index 6b339e644f..12d72c4ab9 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -75,7 +75,7 @@ With `--physical`, the service leases one of the real phones enrolled with it in A service that honours the spec returns the granted device on the lease as `lease.device`: `model`, `osVersion` (the exact OS granted), `imageTag` when one applies, and `physical`. `open --json` prints it under `lease.device`. -The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI releases that lease and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. `details.released` reports whether the release succeeded. If rollback also fails, `details.leaseRollback` names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits allocation ownership evidence leaves an unrecorded lease until expiry when preparation fails. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. +The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI releases that lease and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. `details.released`, when present, reports whether the lease was released. If release throws, rollback retries it once. A failed retry uses the same `details.leaseRollback` shape as a preparation failure: it names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits allocation ownership evidence leaves an unrecorded lease until expiry when preparation fails. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. ## Download captures From 8889d8eba59659eb469f0c30aaded6e1d407cc75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 2 Nov 2026 07:54:50 +0100 Subject: [PATCH 09/11] fix: release discarded allocations without creation evidence --- packages/contracts/src/client-lease.ts | 10 +- ...-connection-proxy-allocate-by-spec.test.ts | 85 ++++++++++++++++- .../__tests__/proxy-lease-device-spec.test.ts | 83 ++++++++++++++++- .../commands/connection-lease-transaction.ts | 92 ++++++++++--------- src/cli/commands/connection-runtime.ts | 8 +- src/cli/commands/proxy-lease-device.ts | 42 +++++---- website/docs/docs/client-api.md | 2 +- website/docs/docs/remote-proxy.md | 2 +- 8 files changed, 247 insertions(+), 77 deletions(-) diff --git a/packages/contracts/src/client-lease.ts b/packages/contracts/src/client-lease.ts index 97b820ffe8..09ef626b60 100644 --- a/packages/contracts/src/client-lease.ts +++ b/packages/contracts/src/client-lease.ts @@ -15,12 +15,10 @@ export type Lease = { deviceKey?: string; clientId?: string; /** - * What an allocate answered about this request: `true` when the daemon created the lease for it, - * `false` when it handed back a lease that already existed — renewed, or already bound to the run. - * The returned lease alone cannot tell the two apart, and a caller that ends a lease must end only - * the one its own request created, so release on `created === true` and on nothing else. Only - * `leases.allocate` answers this; every other read, any daemon older than this field, and every - * lease you did not allocate yourself omit it, and an omitted answer is not permission to release. + * What an allocate answered: `true` when this request created the lease, `false` on reuse. + * Only allocate answers this; heartbeats and older daemons omit creation evidence. + * A discarded allocate response can be released unless `created` is false. + * Rollback after binding requires `created` to be true. */ created?: boolean; /** diff --git a/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts b/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts index f4fe9b5696..25a2f24476 100644 --- a/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts +++ b/src/__tests__/remote-connection-proxy-allocate-by-spec.test.ts @@ -217,10 +217,11 @@ test('a daemon whose lease names no device gets that lease released and the devi fs.rmSync(tempRoot, { recursive: true, force: true }); }); -test('a pre-pick daemon omitting created falls back locally without releasing either allocation', async () => { +test('a pre-pick daemon omitting created releases its keyless lease before binding the local replacement', async () => { const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-pre-pick-'); const requests: LeaseAllocateRequest[] = []; const releases: LeaseReleaseRequest[] = []; + const events: string[] = []; const materialized = await materializeRemoteConnectionForCommand({ host: createTestHostEnvironment(), command: 'open', @@ -229,6 +230,7 @@ test('a pre-pick daemon omitting created falls back locally without releasing ei listDevices: async () => [IPHONE_SIMULATOR], allocate: async (request) => { requests.push(request); + events.push(`allocate-${requests.length}`); const { created: _created, ...lease } = grantedLease(request, { leaseId: `lease-${requests.length}`, }); @@ -236,6 +238,7 @@ test('a pre-pick daemon omitting created falls back locally without releasing ei }, release: async (request) => { releases.push(request); + events.push(`release-${request.leaseId}`); return { released: true }; }, }), @@ -248,7 +251,11 @@ test('a pre-pick daemon omitting created falls back locally without releasing ei const state = readRemoteConnectionState({ stateDir, session: 'adc-proxy' }); assert.equal(state?.leaseId, 'lease-2'); assert.equal(state?.deviceKey, 'ios:mobile:SIM-LOCAL'); - assert.deepEqual(releases, []); + assert.deepEqual( + releases.map((request) => request.leaseId), + ['lease-1'], + ); + assert.deepEqual(events, ['allocate-1', 'release-lease-1', 'allocate-2']); fs.rmSync(tempRoot, { recursive: true, force: true }); }); @@ -301,6 +308,47 @@ test('a keyless lease whose release throws stops the local fallback instead of a fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('an omitted-created keyless discard failure stops fallback and is not retried as owned rollback', async () => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-unknown-discard-fails-'); + const requests: LeaseAllocateRequest[] = []; + const releases: LeaseReleaseRequest[] = []; + await assert.rejects( + materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, device: 'iPhone 17' }, + client: createTestClient({ + listDevices: async () => [IPHONE_SIMULATOR], + allocate: async (request) => { + requests.push(request); + return grantedLease(request, { leaseId: `lease-${requests.length}`, created: undefined }); + }, + release: async (request) => { + releases.push(request); + throw new Error('provider unreachable'); + }, + }), + }), + (error: AppError) => { + assert.equal(error.code, 'COMMAND_FAILED'); + assert.deepEqual(error.details?.leaseRollback, { + leaseId: 'lease-1', + released: false, + reason: 'LEASE_RELEASE_FAILED', + error: 'provider unreachable', + }); + return true; + }, + ); + assert.equal(requests.length, 1); + assert.deepEqual( + releases.map((request) => request.leaseId), + ['lease-1'], + ); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('an already absent keyless lease permits fallback and is released only once', async () => { const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-by-spec-old-noop-'); const requests: LeaseAllocateRequest[] = []; @@ -331,6 +379,39 @@ test('an already absent keyless lease permits fallback and is released only once fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test('an explicitly reused keyless allocation is refused without release or replacement', async () => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-keyless-reused-'); + const requests: LeaseAllocateRequest[] = []; + const releases: LeaseReleaseRequest[] = []; + await assert.rejects( + materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, device: 'iPhone 17' }, + client: createTestClient({ + listDevices: async () => [IPHONE_SIMULATOR], + allocate: async (request) => { + requests.push(request); + return grantedLease(request, { created: false }); + }, + release: async (request) => { + releases.push(request); + return { released: true }; + }, + }), + }), + (error: AppError) => { + assert.equal(error.code, 'UNAUTHORIZED'); + assert.equal(error.details?.reason, 'LEASE_SCOPE_MISMATCH'); + return true; + }, + ); + assert.equal(requests.length, 1); + assert.deepEqual(releases, []); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + fs.rmSync(tempRoot, { recursive: true, force: true }); +}); + test('a renewed lease that names no device is refused and left to its owner', async () => { const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-by-spec-keyless-', { leaseId: 'lease-1', diff --git a/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts b/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts index 36401b92ba..a9d3f7f6f7 100644 --- a/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts +++ b/src/cli/commands/__tests__/proxy-lease-device-spec.test.ts @@ -251,7 +251,7 @@ test('a successful release retry preserves the spec refusal without stale rollba assert.equal(error.details?.reason, 'device-spec-unsupported'); assert.equal(error.details?.leaseRollback, undefined); assert.equal(error.details?.releaseError, undefined); - assert.equal(error.details?.released, undefined); + assert.equal(error.details?.released, true); return true; }, ); @@ -263,6 +263,87 @@ test('a successful release retry preserves the spec refusal without stale rollba fs.rmSync(tempRoot, { recursive: true, force: true }); }); +test.each([ + { created: undefined, releasedIds: ['lease-1'] }, + { created: false, releasedIds: [] }, +] as const)( + 'spec refusal discards an allocate response with created=$created unless explicitly reused', + async ({ created, releasedIds }) => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-spec-discard-'); + const releases: LeaseReleaseRequest[] = []; + await assert.rejects( + materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, osVersion: '>=26' }, + client: createTestClient({ + listDevices: unexpectedCommandCall, + allocate: async (request) => + grantedLease(request, { created, deviceKey: REMOTE_DEVICE_KEY }), + release: async (request) => { + releases.push(request); + return { released: true }; + }, + }), + }), + (error: AppError) => { + assert.equal(error.code, 'UNSUPPORTED_OPERATION'); + assert.equal(error.details?.reason, 'device-spec-unsupported'); + if (created === undefined) assert.equal(error.details?.released, true); + return true; + }, + ); + assert.deepEqual( + releases.map((request) => request.leaseId), + releasedIds, + ); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + fs.rmSync(tempRoot, { recursive: true, force: true }); + }, +); + +test.each([ + { created: undefined, releasedIds: ['lease-1'] }, + { created: false, releasedIds: [] }, +] as const)( + 'identity refusal discards an allocate response with created=$created unless explicitly reused', + async ({ created, releasedIds }) => { + const { tempRoot, stateDir, flags } = proxyWorkspace('agent-device-proxy-identity-discard-'); + const releases: LeaseReleaseRequest[] = []; + await assert.rejects( + materializeRemoteConnectionForCommand({ + host: createTestHostEnvironment(), + command: 'open', + flags: { ...flags, udid: 'SIM-REQUESTED', osVersion: '>=26' }, + client: createTestClient({ + listDevices: unexpectedCommandCall, + allocate: async (request) => + grantedLease(request, { + created, + deviceKey: REMOTE_DEVICE_KEY, + device: GRANTED_DEVICE, + }), + release: async (request) => { + releases.push(request); + return { released: true }; + }, + }), + }), + (error: AppError) => { + assert.equal(error.code, 'INVALID_ARGS'); + assert.equal(error.details?.reason, 'CONNECTION_DEVICE_CONFLICT'); + return true; + }, + ); + assert.deepEqual( + releases.map((request) => request.leaseId), + releasedIds, + ); + assert.equal(readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId, undefined); + fs.rmSync(tempRoot, { recursive: true, force: true }); + }, +); + test('a later open on a live spec lease is served when it names the same spec', async () => { const { tempRoot, flags } = proxyWorkspace('agent-device-proxy-device-spec-same-', { leaseId: 'lease-1', diff --git a/src/cli/commands/connection-lease-transaction.ts b/src/cli/commands/connection-lease-transaction.ts index ac2a43520d..8d0714272d 100644 --- a/src/cli/commands/connection-lease-transaction.ts +++ b/src/cli/commands/connection-lease-transaction.ts @@ -27,63 +27,74 @@ type LeaseRelease = () => Promise<{ released: boolean }>; const LEASE_ROLLBACK_RELEASE_FAILED_REASON = 'LEASE_RELEASE_FAILED'; -/** One newly created lease awaits release or transfer to a durable session record. */ +type TrackedLease = { + leaseId: string; + release: LeaseRelease; + rollbackOnFailure: boolean; +}; + +/** One allocated lease awaits discard or transfer to a durable session record. */ export type LeaseLedger = { /** Release or transfer the previous lease before tracking another allocation. */ - track(leaseId: string, release: LeaseRelease): void; - /** A resolved release ends ownership, including when the daemon reports no such lease. */ - end(leaseId: string): Promise<{ released: boolean }>; + track(lease: TrackedLease): void; + /** Undefined means untracked; a resolved release also ends ownership when no such lease exists. */ + end(leaseId: string, failure?: AppError): Promise<{ released: boolean } | undefined>; transfer(leaseId: string | undefined): void; releaseOwnedOnFailure(failure: unknown): Promise; }; export function createLeaseLedger(): LeaseLedger { - let owned: { leaseId: string; release: LeaseRelease } | undefined; + let owned: TrackedLease | undefined; - async function end(leaseId: string): Promise<{ released: boolean }> { - if (owned?.leaseId !== leaseId) return { released: false }; - const outcome = await owned.release(); - owned = undefined; - return outcome; + async function end( + leaseId: string, + failure?: AppError, + ): Promise<{ released: boolean } | undefined> { + if (owned?.leaseId !== leaseId) return undefined; + try { + const outcome = await owned.release(); + owned = undefined; + if (failure && failure.details?.leaseRollback !== undefined) { + delete failure.details.leaseRollback; + failure.details.released = outcome.released; + } + return outcome; + } catch (error) { + const primary = failure ?? asAppError(error, 'COMMAND_FAILED'); + primary.details = { + ...primary.details, + leaseRollback: { + leaseId, + released: false, + reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, + error: errorMessage(error), + }, + }; + throw primary; + } } return { - track(leaseId, release) { - owned = { leaseId, release }; + track(lease) { + owned = lease; }, end, transfer(leaseId) { if (owned?.leaseId === leaseId) owned = undefined; }, async releaseOwnedOnFailure(failure) { - if (!owned) return failure; - const { leaseId } = owned; + if (!owned?.rollbackOnFailure) return failure; try { - await end(leaseId); + await end(owned.leaseId, asAppError(failure, 'COMMAND_FAILED')); return failure; } catch (error) { - const primary = asAppError(failure, 'COMMAND_FAILED'); - primary.details = { - ...primary.details, - leaseRollback: { - leaseId, - released: false, - reason: LEASE_ROLLBACK_RELEASE_FAILED_REASON, - error: errorMessage(error), - }, - }; - return primary; + return error; } }, }; } -/** - * Brings the connection's lease up to date for this command: renews the one the record names or - * allocates the one it does not, then settles the platform axis that decides which device that lease - * rents. A lease this command created enters `leaseLedger` the moment the provider grants it, before - * any validation or binding here can fail, so the caller's failure path still sees it. - */ +/** Allocation enters the ledger before validation; only confirmed creation permits later rollback. */ export async function acquireConnectionLease(options: { command: string; client: AgentDeviceClient; @@ -324,12 +335,7 @@ function requireRequestedLeaseBackend(flags: CliFlags, command: string): LeaseBa ); } -/** - * A renewed lease is the record's own; only an allocation can bring a new one into this command, and - * only when the daemon says this request created it. A run's repeat allocation gets the run's - * existing lease back — the same answer the record-less caller of another run could get — and that - * lease belongs to whoever holds it, never to this transaction. - */ +/** Omitted creation evidence permits discard, but not rollback after binding. */ async function allocateOrReuseLease( client: AgentDeviceClient, state: RemoteConnectionState, @@ -379,10 +385,12 @@ async function allocateOrReuseLease( ...(allocateBySpec ? requireLeaseDeviceSpec(flags).spec : {}), providerApp: initialApp ?? flags.providerApp, }); - const created = lease.created === true; - if (created) { - // Ownership starts here, before any caller of this function can fail on this lease. - leaseLedger.track(lease.leaseId, leaseAllocationRelease(client, state, leaseBackend, lease)); + if (lease.created !== false) { + leaseLedger.track({ + leaseId: lease.leaseId, + release: leaseAllocationRelease(client, state, leaseBackend, lease), + rollbackOnFailure: lease.created === true, + }); } return { lease, acquired: true }; } diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index a418c63a8d..ddc2de6f13 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -97,12 +97,8 @@ export async function materializeRemoteConnectionForCommand(options: { let nextState = state; let changed = !existingState; - // The ledger owns a lease this command created from the moment allocation returns it, before any - // later step can fail, and holds it until the durable session record names it — the one place - // ownership transfers. Every exit before that transfer ends the lease: a lease no record names is - // invisible to disconnect, and would hold the device until its TTL lapses. A lease this call - // renewed, or one the provider granted to another holder, is named elsewhere already, so this - // path leaves it to whoever does in every case, including a failure. + // Unknown creation evidence permits discard before binding, but only confirmed creation permits + // rollback after a later failure. A committed session record takes over either lease. const leaseLedger = createLeaseLedger(); try { const acquired = await acquireConnectionLease({ diff --git a/src/cli/commands/proxy-lease-device.ts b/src/cli/commands/proxy-lease-device.ts index 075508291f..b677491509 100644 --- a/src/cli/commands/proxy-lease-device.ts +++ b/src/cli/commands/proxy-lease-device.ts @@ -132,7 +132,7 @@ export async function bindLeaseNamedDevice(options: { } if (leased) { const identityConflict = leasedIdentityConflict(options.state, flags, leased); - if (identityConflict) throw identityConflict; + if (identityConflict) throw await releaseRefusedLease(leaseLedger, lease, identityConflict); if (!acquired) { assertLeaseSpecMatchesRequest(options.state, lease, spec, specKeys, leased); await assertLeasedDeviceMatchesRequest(client, options.state, flags, leased); @@ -155,7 +155,8 @@ export async function bindLeaseNamedDevice(options: { }; } if (!acquired) throw unboundLeaseError(options.state, lease); - await leaseLedger.end(lease.leaseId); + const retired = await leaseLedger.end(lease.leaseId); + if (retired === undefined) throw unboundLeaseError(options.state, lease); const resolved = await resolveProxyLeaseStateLocally(options); return { state: resolved.state, @@ -163,31 +164,36 @@ export async function bindLeaseNamedDevice(options: { }; } -/** - * Refuses an unconfirmed spec and attempts release. A thrown release leaves ownership with the - * caller for one rollback retry; only that retry reports failure, so recovery leaves no stale evidence. - */ +/** A refused allocation is discarded unless the daemon explicitly reported reuse. */ async function refuseUnhonouredSpecLease( leaseLedger: LeaseLedger, state: RemoteConnectionState, lease: Lease, keys: readonly string[], ): Promise { - let released: boolean | undefined; + return await releaseRefusedLease( + leaseLedger, + lease, + deviceSpecUnsupportedError( + keys, + 'The proxy backend leased a device without confirming the requested device spec.', + { session: state.session, leaseId: lease.leaseId }, + ), + ); +} + +async function releaseRefusedLease( + leaseLedger: LeaseLedger, + lease: Lease, + failure: AppError, +): Promise { try { - ({ released } = await leaseLedger.end(lease.leaseId)); + const outcome = await leaseLedger.end(lease.leaseId, failure); + if (outcome !== undefined) failure.details = { ...failure.details, released: outcome.released }; } catch { - // The caller retries the owned lease and attaches any failure as details.leaseRollback. + // end() records the failure; only confirmed creation permits the caller's rollback retry. } - return deviceSpecUnsupportedError( - keys, - 'The proxy backend leased a device without confirming the requested device spec.', - { - session: state.session, - leaseId: lease.leaseId, - ...(released === undefined ? {} : { released }), - }, - ); + return failure; } /** diff --git a/website/docs/docs/client-api.md b/website/docs/docs/client-api.md index 3b8095ccf7..25d85339e4 100644 --- a/website/docs/docs/client-api.md +++ b/website/docs/docs/client-api.md @@ -344,7 +344,7 @@ Domain client methods: - `client.sessions.list()`, `stateDir()`, `close()`, `saveScript()`, `artifacts()` - `client.apps.install()`, `reinstall()`, `uninstall()`, `installFromSource()`, `list()`, `open()`, `close()`, `push()`, `triggerEvent()` - `client.materializations.release()` -- `client.leases.allocate()`, `heartbeat()`, `release()`. Pass `retainOnClose: true` to `allocate()` when you release the lease yourself; session `close` then leaves it active until `release()`, expiry, or daemon shutdown. The returned lease has `retainOnClose: true` only when the daemon honored it. Keep heartbeating a retained lease between sessions: it expires after its `ttlMs` like any other lease. Pass `leaseId` (16 to 128 hex characters) to `allocate()` to name a new lease with an id the daemon has never issued, or to get back your live lease with that id when you retry with the same tenant, run, client, lease backend, and provider (and no device, or the leased one), then use the returned `leaseId`: a run that already holds a lease gets that one back, and an older daemon mints its own. `allocate()` also tells you which of those you got, on the returned lease's `created`: `true` when your request created that lease, `false` when it was already there, and omitted on `heartbeat()` or when an older daemon does not support ownership evidence. Release a lease only when `created` is `true`. A lease you did not create belongs to whoever did, and an omitted `created` is not permission to release one. Against an older daemon, the CLI can still bind the allocated device, but cannot roll back an unrecorded lease after preparation fails; that lease remains until expiry. `allocate()` with a `leaseProvider` the daemon has no runtime for fails with `UNSUPPORTED_OPERATION`, reason `provider-not-registered`, and `details.registeredProviders`; no lease is recorded. Leases with no provider, `proxy`, or `cloud` need no runtime. +- `client.leases.allocate()`, `heartbeat()`, `release()`. Pass `retainOnClose: true` to `allocate()` when you release the lease yourself; session `close` then leaves it active until `release()`, expiry, or daemon shutdown. The returned lease has `retainOnClose: true` only when the daemon honored it. Keep heartbeating a retained lease between sessions: it expires after its `ttlMs` like any other lease. Pass `leaseId` (16 to 128 hex characters) to `allocate()` to name a new lease with an id the daemon has never issued, or to get back your live lease with that id when you retry with the same tenant, run, client, lease backend, and provider (and no device, or the leased one), then use the returned `leaseId`: a run that already holds a lease gets that one back, and an older daemon mints its own. `allocate()` also tells you which of those you got, on the returned lease's `created`: `true` when your request created that lease, `false` when it was already there, and omitted on `heartbeat()` or when an older daemon does not support ownership evidence. The CLI releases an allocate response it discards before recording (a spec refusal, identity refusal, or keyless replacement), unless `created` is `false`. After binding, rollback on a later preparation failure requires `created:true`. Against an older daemon that omits `created`, refused or replaced leases are released, but a bound, unrecorded lease remains until expiry if preparation fails. `allocate()` with a `leaseProvider` the daemon has no runtime for fails with `UNSUPPORTED_OPERATION`, reason `provider-not-registered`, and `details.registeredProviders`; no lease is recorded. Leases with no provider, `proxy`, or `cloud` need no runtime. - `client.metro.prepare()`, `reload()` - `client.capture.snapshot()`, `screenshot()`, `diff()` - `client.interactions.gesture()`, `click()`, `press()`, `longPress()`, `swipe()`, `pan()`, `drag()`, `fling()`, `swipeGesture()`, `focus()`, `type()`, `fill()`, `scroll()`, `pinch()`, `rotateGesture()`, `transformGesture()`, `tapGesture()`, `get()`, `is()`, `find()` diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index 12d72c4ab9..75a84f8f80 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -75,7 +75,7 @@ With `--physical`, the service leases one of the real phones enrolled with it in A service that honours the spec returns the granted device on the lease as `lease.device`: `model`, `osVersion` (the exact OS granted), `imageTag` when one applies, and `physical`. `open --json` prints it under `lease.device`. -The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI releases that lease and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. `details.released`, when present, reports whether the lease was released. If release throws, rollback retries it once. A failed retry uses the same `details.leaseRollback` shape as a preparation failure: it names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits allocation ownership evidence leaves an unrecorded lease until expiry when preparation fails. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. +The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI discards that allocate response and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. It releases a discarded lease unless the daemon explicitly reported `created:false` (reuse). This also applies when a grant conflicts with `--udid` or `--serial`, or when a keyless lease is replaced by the locally selected device. `details.released`, when present, reports whether the lease was released. If release throws and the daemon confirmed `created:true`, rollback retries it once. A retry that releases the lease records `released:true`. A failed discard or rollback uses one `details.leaseRollback` shape: it names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits creation evidence still has refused or replaced leases released. Once its grant is bound, a later preparation failure leaves that unrecorded lease until expiry because rollback requires `created:true`. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. ## Download captures From 10f559900140fac6fd5c50477a120691d6b84f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 2 Nov 2026 08:27:10 +0100 Subject: [PATCH 10/11] docs: limit lease release retry to discarded allocations --- website/docs/docs/remote-proxy.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index 75a84f8f80..a7ba0f84a6 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -75,7 +75,7 @@ With `--physical`, the service leases one of the real phones enrolled with it in A service that honours the spec returns the granted device on the lease as `lease.device`: `model`, `osVersion` (the exact OS granted), `imageTag` when one applies, and `physical`. `open --json` prints it under `lease.device`. -The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI discards that allocate response and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. It releases a discarded lease unless the daemon explicitly reported `created:false` (reuse). This also applies when a grant conflicts with `--udid` or `--serial`, or when a keyless lease is replaced by the locally selected device. `details.released`, when present, reports whether the lease was released. If release throws and the daemon confirmed `created:true`, rollback retries it once. A retry that releases the lease records `released:true`. A failed discard or rollback uses one `details.leaseRollback` shape: it names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits creation evidence still has refused or replaced leases released. Once its grant is bound, a later preparation failure leaves that unrecorded lease until expiry because rollback requires `created:true`. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. +The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI discards that allocate response and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. It releases a discarded lease unless the daemon explicitly reported `created:false` (reuse). This also applies when a grant conflicts with `--udid` or `--serial`, or when a keyless lease is replaced by the locally selected device. `details.released`, when present, reports whether the lease was released. If release throws while discarding a refused or conflicting allocation and the daemon confirmed `created:true`, the command retries that release once. A retry that releases the lease records `released:true`. A failed discard or rollback uses one `details.leaseRollback` shape: it names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits creation evidence still has refused or replaced leases released. Once its grant is bound, a later preparation failure leaves that unrecorded lease until expiry because rollback requires `created:true`. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. ## Download captures From 0c63ddcba566b10af02543b5cf5191681b14e189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 2 Nov 2026 08:46:41 +0100 Subject: [PATCH 11/11] fix: scope allocation evidence and protect the lease ledger --- .../client/src/__tests__/lease-client.test.ts | 34 ++++++ packages/client/src/lease-client.ts | 20 +-- .../connection-lease-transaction.test.ts | 33 +++++ .../commands/connection-lease-transaction.ts | 11 ++ .../connection-runtime-remote-auth.test.ts | 115 ++++++++++++++++++ website/docs/docs/remote-proxy.md | 2 +- 6 files changed, 204 insertions(+), 11 deletions(-) create mode 100644 src/cli/commands/connection-lease-transaction.test.ts create mode 100644 src/cli/commands/connection-runtime-remote-auth.test.ts diff --git a/packages/client/src/__tests__/lease-client.test.ts b/packages/client/src/__tests__/lease-client.test.ts index ec20494034..a14806dac8 100644 --- a/packages/client/src/__tests__/lease-client.test.ts +++ b/packages/client/src/__tests__/lease-client.test.ts @@ -129,6 +129,40 @@ test('allocate sends the lease id named on the call, never the configured one', assert.equal(lease.leaseId, 'abcdef0123456789'); }); +test.each([ + { operation: 'allocate', reused: false, created: true }, + { operation: 'allocate', reused: true, created: false }, + { operation: 'heartbeat', reused: false, created: undefined }, + { operation: 'heartbeat', reused: true, created: undefined }, +] as const)( + '$operation reads creation evidence only from an allocate response (reused=$reused)', + async ({ operation, reused, created }) => { + const client = createAgentDeviceClient( + {}, + { + transport: async () => ({ + ok: true, + data: { + reused, + lease: { + leaseId: 'lease-a', + tenantId: 'tenant-a', + runId: 'run-a', + backend: 'ios-instance', + }, + }, + }), + }, + ); + const lease = await client.leases[operation]({ + tenant: 'tenant-a', + runId: 'run-a', + leaseId: 'lease-a', + }); + assert.equal(lease.created, created); + }, +); + for (const operation of ['allocate', 'heartbeat'] as const) { const validLease = { leaseId: 'lease-a', diff --git a/packages/client/src/lease-client.ts b/packages/client/src/lease-client.ts index a594723cd6..c0275c891a 100644 --- a/packages/client/src/lease-client.ts +++ b/packages/client/src/lease-client.ts @@ -17,13 +17,16 @@ export function createLeaseClient( const control = async (positionals: string[], options?: InternalRequestOptions) => await execute(INTERNAL_COMMANDS.humanControl, positionals, options); return { - allocate: async (options) => - normalizeLease( - await execute(INTERNAL_COMMANDS.leaseAllocate, [], { - ...options, - leaseId: options.leaseId, - }), - ), + allocate: async (options) => { + const data = await execute(INTERNAL_COMMANDS.leaseAllocate, [], { + ...options, + leaseId: options.leaseId, + }); + return { + ...normalizeLease(data), + ...(typeof data.reused === 'boolean' ? { created: !data.reused } : {}), + }; + }, heartbeat: async (options) => normalizeLease(await execute(INTERNAL_COMMANDS.leaseHeartbeat, [], options)), release: async (options) => { @@ -94,9 +97,6 @@ function normalizeLease(data: Record): Lease { leaseProvider: readOptionalString(rawLease, 'leaseProvider'), clientId: readOptionalString(rawLease, 'clientId'), deviceKey: readOptionalString(rawLease, 'deviceKey'), - // Only an allocate answers `reused`; every other read, and any daemon older than the field, - // omits it, and the caller keeps `created` unset rather than guessing who made the lease. - ...(typeof data.reused === 'boolean' ? { created: !data.reused } : {}), ...withLeaseDevice(readLeaseDevice(rawLease.device)), ...(rawLease.retainOnClose === true ? { retainOnClose: true } : {}), createdAt: typeof rawLease.createdAt === 'number' ? rawLease.createdAt : undefined, diff --git a/src/cli/commands/connection-lease-transaction.test.ts b/src/cli/commands/connection-lease-transaction.test.ts new file mode 100644 index 0000000000..fd01ae409e --- /dev/null +++ b/src/cli/commands/connection-lease-transaction.test.ts @@ -0,0 +1,33 @@ +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { createLeaseLedger } from './connection-lease-transaction.ts'; + +test('rejecting a second tracked allocation preserves the first lease for rollback', async () => { + const ledger = createLeaseLedger(); + const released: string[] = []; + ledger.track({ + leaseId: 'first', + rollbackOnFailure: true, + release: async () => { + released.push('first'); + return { released: true }; + }, + }); + assert.throws( + () => + ledger.track({ + leaseId: 'second', + rollbackOnFailure: true, + release: async () => { + released.push('second'); + return { released: true }; + }, + }), + (error: unknown) => + error instanceof AppError && error.details?.reason === 'LEASE_LEDGER_OVERWRITE', + ); + const primary = new AppError('COMMAND_FAILED', 'materialization failed'); + assert.equal(await ledger.releaseOwnedOnFailure(primary), primary); + assert.deepEqual(released, ['first']); +}); diff --git a/src/cli/commands/connection-lease-transaction.ts b/src/cli/commands/connection-lease-transaction.ts index 8d0714272d..9a43e09a05 100644 --- a/src/cli/commands/connection-lease-transaction.ts +++ b/src/cli/commands/connection-lease-transaction.ts @@ -76,6 +76,17 @@ export function createLeaseLedger(): LeaseLedger { return { track(lease) { + if (owned !== undefined) { + throw new AppError( + 'COMMAND_FAILED', + 'Release or transfer the tracked lease before another allocation.', + { + reason: 'LEASE_LEDGER_OVERWRITE', + leaseId: owned.leaseId, + trackedLeaseId: lease.leaseId, + }, + ); + } owned = lease; }, end, diff --git a/src/cli/commands/connection-runtime-remote-auth.test.ts b/src/cli/commands/connection-runtime-remote-auth.test.ts new file mode 100644 index 0000000000..a02dc12b37 --- /dev/null +++ b/src/cli/commands/connection-runtime-remote-auth.test.ts @@ -0,0 +1,115 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { test, vi } from 'vitest'; +import { createAgentDeviceClient } from '@agent-device/client'; +import { AppError } from '@agent-device/kernel/errors'; +import { isInactiveLeaseError } from '@agent-device/contracts/lease-scope'; +import { createTestHostEnvironment } from '@agent-device/test-kit'; +import { + connectionWorkspace, + seedConnectionState, +} from '../../__tests__/remote-connection.fixtures.ts'; +import { startPublicDaemonRuntime } from '../../__tests__/test-utils/public-daemon-runtime.ts'; +import { readRemoteConnectionState } from '@agent-device/remote/remote-connection-state'; +import { materializeRemoteConnectionForCommand } from './connection-runtime.ts'; + +test('an authenticated remote rollback inherits routing and ends only the replacement of an inactive recorded lease', async () => { + const daemon = await startPublicDaemonRuntime({ serverMode: 'http' }); + const { tempRoot, stateDir, remoteConfigPath } = connectionWorkspace( + 'agent-device-authenticated-rollback-', + ); + const host = createTestHostEnvironment(); + const config = { + stateDir, + daemonBaseUrl: daemon.baseUrl, + daemonAuthToken: daemon.runtime.token, + daemonTransport: 'http' as const, + daemonServerMode: 'http' as const, + tenant: 'route-tenant', + runId: 'route-run', + session: 'remote-auth', + }; + const scope = { + tenant: config.tenant, + runId: config.runId, + leaseBackend: 'android-instance' as const, + }; + const client = createAgentDeviceClient(config, { host }); + try { + const first = await client.leases.allocate(scope); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: daemon.baseUrl })); + seedConnectionState({ + stateDir, + state: { + session: config.session, + remoteConfigPath, + tenant: config.tenant, + runId: config.runId, + daemon: { baseUrl: daemon.baseUrl, transport: 'http', serverMode: 'http' }, + leaseId: first.leaseId, + leaseBackend: 'android-instance', + platform: 'android', + }, + }); + await client.leases.release({ ...scope, leaseId: first.leaseId }); + await assert.rejects( + client.leases.allocate({ ...scope, leaseId: first.leaseId }), + (error: unknown) => error instanceof AppError && error.details?.reason === 'LEASE_ID_IN_USE', + ); + const bound = createAgentDeviceClient({ ...config, leaseId: first.leaseId }, { host }); + const allocate = vi.spyOn(bound.leases, 'allocate'); + const release = vi.spyOn(bound.leases, 'release'); + const primary = new AppError('COMMAND_FAILED', 'Metro preparation failed', { + reason: 'METRO_PREPARE_FAILED', + }); + vi.spyOn(bound.metro, 'prepare').mockRejectedValue(primary); + await assert.rejects( + materializeRemoteConnectionForCommand({ + host, + command: 'open', + client: bound, + flags: { + ...config, + json: true, + help: false, + version: false, + remoteConfig: remoteConfigPath, + platform: 'android', + metroPublicBaseUrl: 'https://metro.example.test', + }, + }), + (error: unknown) => error === primary, + ); + assert.equal(allocate.mock.results.length, 1); + const replacement = await allocate.mock.results[0]!.value; + assert.equal(replacement.created, true); + assert.notEqual(replacement.leaseId, first.leaseId); + assert.deepEqual( + release.mock.calls.map(([request]) => request.leaseId), + [replacement.leaseId], + ); + assert.equal(primary.details?.leaseRollback, undefined); + await assert.rejects( + bound.leases.heartbeat({ ...scope, leaseId: replacement.leaseId }), + isInactiveLeaseError, + ); + const unauthorized = createAgentDeviceClient( + { ...config, daemonAuthToken: 'wrong-test-token' }, + { host }, + ); + await assert.rejects( + unauthorized.leases.release({ ...scope, leaseId: replacement.leaseId }), + (error: unknown) => error instanceof AppError && error.code === 'UNAUTHORIZED', + ); + assert.equal( + readRemoteConnectionState({ stateDir, session: config.session })?.leaseId, + first.leaseId, + ); + assert.equal(fs.existsSync(path.join(stateDir, 'daemon.json')), false); + } finally { + vi.restoreAllMocks(); + await daemon.close(); + fs.rmSync(tempRoot, { recursive: true, force: true }); + } +}); diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index a7ba0f84a6..18975a9141 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -75,7 +75,7 @@ With `--physical`, the service leases one of the real phones enrolled with it in A service that honours the spec returns the granted device on the lease as `lease.device`: `model`, `osVersion` (the exact OS granted), `imageTag` when one applies, and `physical`. `open --json` prints it under `lease.device`. -The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI discards that allocate response and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. It releases a discarded lease unless the daemon explicitly reported `created:false` (reuse). This also applies when a grant conflicts with `--udid` or `--serial`, or when a keyless lease is replaced by the locally selected device. `details.released`, when present, reports whether the lease was released. If release throws while discarding a refused or conflicting allocation and the daemon confirmed `created:true`, the command retries that release once. A retry that releases the lease records `released:true`. A failed discard or rollback uses one `details.leaseRollback` shape: it names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits creation evidence still has refused or replaced leases released. Once its grant is bound, a later preparation failure leaves that unrecorded lease until expiry because rollback requires `created:true`. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. +The daemon itself cannot pick a device by these keys, so it refuses a named key instead of ignoring it. A local daemon, or one behind a plain `agent-device proxy`, fails such a request with `UNSUPPORTED_OPERATION`, `details.reason: "device-spec-unsupported"`, and `details.keys` naming the keys it got, and records no lease. When a service returns a lease without `lease.device`, the service did not understand the spec. The CLI discards that allocate response and fails with `UNSUPPORTED_OPERATION` and `details.reason: "device-spec-unsupported"`. It releases a discarded lease unless the daemon explicitly reported `created:false` (reuse). This also applies when a grant conflicts with `--udid` or `--serial`, or when a keyless lease is replaced by the locally selected device. `details.released`, when present, reports whether the lease was released. If release throws while discarding a refused or conflicting allocation and the daemon confirmed `created:true`, the command retries that release once. A retry that releases the lease records `released:true`. Keyless fallback stops if discarding the replaced lease throws, to avoid leaving an unnamed lease while allocating another. A failed discard or rollback uses one `details.leaseRollback` shape: it names the lease and carries `released: false`, reason `LEASE_RELEASE_FAILED`, and the release error alongside the original command error. An older daemon that omits creation evidence still has refused or replaced leases released. Once its grant is bound, a later preparation failure leaves that unrecorded lease until expiry because rollback requires `created:true`. While a lease is active, a later command that names a different value for any of the three keys fails with reason `CONNECTION_DEVICE_CONFLICT`; a key the command leaves out keeps the lease's value. Run `connect --force` to lease by another spec. ## Download captures