diff --git a/docs/adr/0007-remote-device-leases.md b/docs/adr/0007-remote-device-leases.md index ce9d632819..b69a1f181e 100644 --- a/docs/adr/0007-remote-device-leases.md +++ b/docs/adr/0007-remote-device-leases.md @@ -62,6 +62,20 @@ in-flight work and human-control holds still keep a lease from expiring. Once daemon shutdown begins, no new lease-end teardown starts, and shutdown waits for any that already started before it tears sessions down itself. +The daemon ends a session's lease when a request closes that session, unless the lease was +allocated with `retainOnClose`. The reply to that request reports each lease it ended once, at +response level, as `endedLeases: [{ leaseId, outcome: 'released' }]`: in `data` on success, in +`error.details` on failure, including a close that failed after the release. This holds for every +request that can close a session: `close`, a `batch` close step, a `replay` or `test` script's +`close`, and the session cleanup `test` runs after each script. Nested requests record on the +outermost request's ledger, so a step reply never repeats it. A close that finds its lease already +inactive reports it as ended. A client never sends a release after such a request. A client that +stores the lease id forgets it when the reply names it, and a gateway that relays the reply reads +the lease end from it. Close and lease-end teardown take the same execution locks, so whichever +runs second finds the session gone: a close after a lease end has no session and ends no lease, and +a client that still stores that lease id finds it inactive on its next heartbeat and allocates a +new lease under a fresh id. + ## Consequences Device contention can fail before platform execution with an explicit diff --git a/packages/contracts/src/client-lease.ts b/packages/contracts/src/client-lease.ts index c7faf6eca1..57628f61e8 100644 --- a/packages/contracts/src/client-lease.ts +++ b/packages/contracts/src/client-lease.ts @@ -21,6 +21,15 @@ export type Lease = { expiresAt?: number; }; +/** + * A session lease a daemon request ended. A reply to a request that ended leases lists them once in + * `endedLeases` at response level: in `data` on success, in `error.details` on failure. A `close`, + * a `batch` close step, a `replay` or `test` script's close, and `test` session cleanup each end the + * lease they close with, unless it was allocated with `retainOnClose`. A daemon older than this + * field omits it. + */ +export type EndedLease = { leaseId: string; outcome: 'released' }; + export type LeaseOptions = AgentDeviceRequestOverrides & AgentDeviceSelectionOptions & { ttlMs?: number; diff --git a/packages/contracts/src/facades/client.ts b/packages/contracts/src/facades/client.ts index f7ec417f78..f8d5b97d03 100644 --- a/packages/contracts/src/facades/client.ts +++ b/packages/contracts/src/facades/client.ts @@ -67,6 +67,7 @@ export type { } from '../client-gesture.ts'; export type { CloudArtifactsOptions, + EndedLease, HumanControlHold, HumanControlHoldOptions, HumanControlHoldScope, diff --git a/packages/contracts/src/lease-scope.ts b/packages/contracts/src/lease-scope.ts index a1dec7d4d7..579830f6da 100644 --- a/packages/contracts/src/lease-scope.ts +++ b/packages/contracts/src/lease-scope.ts @@ -1,7 +1,8 @@ -import type { LeaseBackend } from '@agent-device/kernel/contracts'; +import type { DaemonResponse, LeaseBackend } from '@agent-device/kernel/contracts'; import { stripUndefined } from '@agent-device/kernel/record'; import type { CloudProviderProfileFields } from './remote-config-fields.ts'; import type { CommandFlags } from './command-flags.ts'; +import type { EndedLease } from './client-lease.ts'; const PROXY_LEASE_PROVIDER = 'proxy'; @@ -343,6 +344,18 @@ export function isInactiveLeaseError(error: unknown): boolean { ); } +/** The leases a daemon reply says its request ended; a malformed entry is no ended lease. */ +export function readEndedLeases(response: DaemonResponse): EndedLease[] { + const body = response.ok ? response.data : response.error.details; + const entries = body?.endedLeases; + if (!Array.isArray(entries)) return []; + return entries.flatMap((entry: unknown) => { + if (typeof entry !== 'object' || entry === null) return []; + const { leaseId, outcome } = entry as Readonly<{ leaseId?: unknown; outcome?: unknown }>; + return typeof leaseId === 'string' && outcome === 'released' ? [{ leaseId, outcome }] : []; + }); +} + function leaseScopeToScopedRequest(scope: LeaseScope): LeaseScopedRequestScope { return stripUndefined({ leaseId: scope.leaseId ?? '', diff --git a/packages/daemon/src/core/__tests__/request-ended-leases.test.ts b/packages/daemon/src/core/__tests__/request-ended-leases.test.ts new file mode 100644 index 0000000000..15fe160c70 --- /dev/null +++ b/packages/daemon/src/core/__tests__/request-ended-leases.test.ts @@ -0,0 +1,146 @@ +import { createTestDeviceInventoryGateways } from '@agent-device/device-selection/inventory-gateways-fixtures'; +import fs from 'node:fs'; +import path from 'node:path'; +import { expect, test } from 'vitest'; +import { mkdtempForTestSync } from '@agent-device/test-kit/tmp-dir'; +import { replayScriptSourceBundleFor } from '@agent-device/replay-port/replay-script-source-fixtures'; +import { makeSessionStore } from '../../__tests__/support/store-factory.ts'; +import { makeIosSession } from '../../__tests__/support/session-factories.ts'; +import { + createRequestHandler, + lifecycleDeviceRuntimeGateway, +} from '../../__tests__/support/test-device-runtime-gateway.ts'; +import { fakeCreateCommandSurface } from '../../__tests__/support/command-surface-fake.ts'; +import { LeaseRegistry } from '../../resources/lease-registry.ts'; +import type { DaemonRequest } from '../daemon-request.ts'; +import { + type EndedLeaseLedger, + onDelegatingLedger, + recordEndedLease, + reportEndedLeases, +} from '../request-ended-leases.ts'; + +const SESSION = 'default'; + +function leasedDaemon() { + const root = mkdtempForTestSync('agent-device-ended-leases-'); + const sessionStore = makeSessionStore('agent-device-ended-leases-store-'); + const leaseRegistry = new LeaseRegistry(); + const lease = leaseRegistry.allocateLease({ + tenantId: 'tenant-a', + runId: 'run-1', + leaseProvider: 'proxy', + clientId: 'client-a', + deviceKey: 'ios:sim-1', + }); + sessionStore.publish( + SESSION, + makeIosSession(SESSION, { + lease: { + leaseId: lease.leaseId, + tenantId: lease.tenantId, + runId: lease.runId, + leaseProvider: lease.leaseProvider, + clientId: lease.clientId, + deviceKey: lease.deviceKey, + }, + }), + ); + const handler = createRequestHandler({ + createCommandSurface: fakeCreateCommandSurface, + logPath: path.join(root, 'daemon.log'), + token: 'test-token', + sessionStore, + leaseRegistry, + deviceInventoryGateways: createTestDeviceInventoryGateways(), + deviceRuntimeGateway: lifecycleDeviceRuntimeGateway, + trackDownloadableArtifact: () => 'artifact-id', + }); + return { + root, + handler, + leaseRegistry, + endedLeases: [{ leaseId: lease.leaseId, outcome: 'released' }], + }; +} + +function request(command: string, positionals: string[] = [], flags = {}): DaemonRequest { + return { token: 'test-token', session: SESSION, command, positionals, flags }; +} + +test('a close reports the lease it ended once, at response level', async () => { + const { handler, leaseRegistry, endedLeases } = leasedDaemon(); + + const response = await handler(request('close')); + + expect(response).toMatchObject({ ok: true, data: { endedLeases } }); + expect(leaseRegistry.listActiveLeases()).toHaveLength(0); +}); + +test('a batch reports the lease its close step ended on the batch reply only', async () => { + const { handler, endedLeases } = leasedDaemon(); + + const response = await handler( + request('batch', [], { batchSteps: [{ command: 'close', positionals: [], flags: {} }] }), + ); + + expect(response).toMatchObject({ + ok: true, + data: { endedLeases, results: [{ command: 'close', ok: true }] }, + }); + expect(response).not.toMatchObject({ data: { results: [{ data: { endedLeases } }] } }); +}); + +test('a batch that fails after its close step reports the ended lease in the error details', async () => { + const { handler, endedLeases } = leasedDaemon(); + + const response = await handler( + request('batch', [], { + batchSteps: [ + { command: 'close', positionals: [], flags: {} }, + { command: 'snapshot', positionals: [], flags: {} }, + ], + }), + ); + + expect(response).toMatchObject({ ok: false, error: { details: { endedLeases } } }); +}); + +test('a request that ended no lease replies without endedLeases', async () => { + const response = await reportEndedLeases(request('snapshot'), async () => ({ + ok: true, + data: {}, + })); + + expect(response).toEqual({ ok: true, data: {} }); +}); + +test('a nested request records on its delegating ledger and reports nothing itself', async () => { + const ledger: EndedLeaseLedger = []; + const nested = onDelegatingLedger(request('close'), { + ...request('batch'), + internal: { endedLeases: ledger }, + }); + + const response = await reportEndedLeases(nested, async (req) => { + recordEndedLease(req, 'lease-1'); + return { ok: true, data: {} }; + }); + + expect(response).toEqual({ ok: true, data: {} }); + expect(ledger).toEqual([{ leaseId: 'lease-1', outcome: 'released' }]); +}); + +test('a replay reports the lease its script close ended', async () => { + const { root, handler, endedLeases } = leasedDaemon(); + const scriptPath = path.join(root, 'close.ad'); + fs.writeFileSync(scriptPath, 'close\n'); + + const response = await handler( + request('replay', [scriptPath], { + replayScriptSource: replayScriptSourceBundleFor(scriptPath), + }), + ); + + expect(response).toMatchObject({ ok: true, data: { endedLeases } }); +}); diff --git a/packages/daemon/src/core/daemon-request.ts b/packages/daemon/src/core/daemon-request.ts index bd9063528f..735f1ab478 100644 --- a/packages/daemon/src/core/daemon-request.ts +++ b/packages/daemon/src/core/daemon-request.ts @@ -9,6 +9,7 @@ import type { } from '@agent-device/kernel/contracts'; import type { DaemonWireRequest } from '@agent-device/contracts/command'; import type { RequestDispatchLedger } from './request-dispatch-ledger.ts'; +import type { EndedLeaseLedger } from './request-ended-leases.ts'; /** * The daemon's own request and response vocabulary: the wire shape from `@agent-device/contracts/command` (`DaemonWireRequest`) @@ -60,6 +61,11 @@ type DaemonRequestInternal = ReplayDispatchOptions & { * delegated it so those mutations count toward that request's disclosure too. */ dispatchLedger?: RequestDispatchLedger; + /** + * The leases the outermost request and its nested requests ended. Only the request that created + * it reports it, once, on its reply. + */ + endedLeases?: EndedLeaseLedger; }; /** diff --git a/packages/daemon/src/core/request-ended-leases.ts b/packages/daemon/src/core/request-ended-leases.ts new file mode 100644 index 0000000000..6e4747c3e6 --- /dev/null +++ b/packages/daemon/src/core/request-ended-leases.ts @@ -0,0 +1,40 @@ +import type { EndedLease } from '@agent-device/contracts/client'; +import type { DaemonInvokeFn, DaemonRequest, DaemonResponse } from './daemon-request.ts'; + +/** The session leases one request ended, its nested requests' included. */ +export type EndedLeaseLedger = EndedLease[]; + +/** Records a lease `req` ended on the ledger of the outermost request it runs under. */ +export function recordEndedLease(req: DaemonRequest, leaseId: string): void { + req.internal?.endedLeases?.push({ leaseId, outcome: 'released' }); +} + +/** + * Runs `handle` for an outermost request on a ledger of its own and reports the leases it ended on + * its reply. A nested request already carries its delegating request's ledger and reports nothing. + */ +export async function reportEndedLeases( + req: DaemonRequest, + handle: (req: DaemonRequest) => Promise, +): Promise { + if (req.internal?.endedLeases) return await handle(req); + const endedLeases: EndedLeaseLedger = []; + const response = await handle({ ...req, internal: { ...req.internal, endedLeases } }); + if (endedLeases.length === 0) return response; + if (response.ok) return { ok: true, data: { ...response.data, endedLeases } }; + return { + ok: false, + error: { ...response.error, details: { ...response.error.details, endedLeases } }, + }; +} + +/** `req` recording its ended leases on the ledger of `parent`, the request that delegated it. */ +export function onDelegatingLedger(req: DaemonRequest, parent: DaemonRequest): DaemonRequest { + const endedLeases = parent.internal?.endedLeases; + return endedLeases ? { ...req, internal: { ...req.internal, endedLeases } } : req; +} + +/** Each nested request `invoke` runs records its ended leases on the ledger of `parent`. */ +export function handDownEndedLeases(invoke: DaemonInvokeFn, parent: DaemonRequest): DaemonInvokeFn { + return async (req) => await invoke(onDelegatingLedger(req, parent)); +} diff --git a/packages/daemon/src/server/request-router.ts b/packages/daemon/src/server/request-router.ts index 43af03b157..3d8441c89a 100644 --- a/packages/daemon/src/server/request-router.ts +++ b/packages/daemon/src/server/request-router.ts @@ -92,6 +92,7 @@ import { refusedBeforeDispatch, } from '../execution/request-dispatch-disclosure.ts'; import { recordNestedRequests } from '../core/request-dispatch-ledger.ts'; +import { handDownEndedLeases, reportEndedLeases } from '../core/request-ended-leases.ts'; import type { AndroidObservationAdapter } from '@agent-device/contracts/android-observation'; import type { CreateDaemonCommandSurface } from '@agent-device/contracts/daemon-command-surface'; import type { PlatformResourceCleanup } from '../core/platform-resource-cleanup.ts'; @@ -213,6 +214,10 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { : deps.deviceInventoryGateways; async function handleRequest(req: DaemonRequest): Promise { + return await reportEndedLeases(req, handleRequestOnLedger); + } + + async function handleRequestOnLedger(req: DaemonRequest): Promise { const start = Date.now(); const debug = Boolean(req.meta?.debug || req.flags?.verbose); return await withDiagnosticsScope( @@ -385,7 +390,10 @@ export function createRequestHandler(deps: RequestRouterDeps): DaemonInvokeFn { cloudArtifactProvider, endSessionsForLease, providerAppCatalog, - invoke: recordNestedRequests(handleRequest, dispatchLedger), + invoke: handDownEndedLeases( + recordNestedRequests(handleRequest, dispatchLedger), + lockedScope.req, + ), invokeReplayAction: allowReplayActions ? recordNestedRequests( createReplayScopedActionInvoker(lockedScope, providerScope), diff --git a/packages/daemon/src/sessions/__tests__/lease-lifecycle.test.ts b/packages/daemon/src/sessions/__tests__/lease-lifecycle.test.ts index 692416ca6b..ef2e3b8d2d 100644 --- a/packages/daemon/src/sessions/__tests__/lease-lifecycle.test.ts +++ b/packages/daemon/src/sessions/__tests__/lease-lifecycle.test.ts @@ -118,7 +118,7 @@ test('releaseSessionLease releases with the stored session owner scope', async ( }, }); - const provider = await releaseSessionLease({ + const released = await releaseSessionLease({ session, leaseRegistry, leaseLifecycleProvider: { @@ -127,7 +127,7 @@ test('releaseSessionLease releases with the stored session owner scope', async ( }); expect(leaseRegistry.listActiveLeases()).toHaveLength(0); - expect(provider).toEqual({ provider: 'proxy' }); + expect(released).toEqual({ endedLeaseId: lease.leaseId, providerData: { provider: 'proxy' } }); }); test('releaseSessionLease leaves a retainOnClose lease and its provider device alone', async () => { @@ -149,13 +149,13 @@ test('releaseSessionLease leaves a retainOnClose lease and its provider device a }); const release = vi.fn(async () => ({ released: true })); - const provider = await releaseSessionLease({ + const retained = await releaseSessionLease({ session, leaseRegistry, leaseLifecycleProvider: { release }, }); - expect(provider).toBeUndefined(); + expect(retained).toEqual({}); expect(release).not.toHaveBeenCalled(); expect(leaseRegistry.listActiveLeases()).toHaveLength(1); expect(vi.mocked(emitDiagnostic)).toHaveBeenCalledWith({ @@ -165,6 +165,19 @@ test('releaseSessionLease leaves a retainOnClose lease and its provider device a }); }); +test('releaseSessionLease ends no lease for a session that holds none', async () => { + const release = vi.fn(async () => ({ released: true })); + + const outcome = await releaseSessionLease({ + session: makeIosSession('default'), + leaseRegistry: new LeaseRegistry(), + leaseLifecycleProvider: { release }, + }); + + expect(outcome).toEqual({}); + expect(release).not.toHaveBeenCalled(); +}); + test('releaseSessionLease retains provider session ownership for artifact lookup', async () => { const leaseRegistry = new LeaseRegistry(); const lease = leaseRegistry.allocateLease({ diff --git a/packages/daemon/src/sessions/handlers/__tests__/session-replay-command.test.ts b/packages/daemon/src/sessions/handlers/__tests__/session-replay-command.test.ts index 24af32e8ec..8aba235f06 100644 --- a/packages/daemon/src/sessions/handlers/__tests__/session-replay-command.test.ts +++ b/packages/daemon/src/sessions/handlers/__tests__/session-replay-command.test.ts @@ -1,3 +1,4 @@ +import fs from 'node:fs'; import path from 'node:path'; import { expect, test } from 'vitest'; import { LeaseRegistry } from '../../../resources/lease-registry.ts'; @@ -13,6 +14,13 @@ import { daemonPlatformServicesFixture } from '../../../__tests__/support/platfo import { makeSessionStore } from '../../../__tests__/support/handlers/session-test-harness.ts'; import { mkdtempForTestSync } from '@agent-device/test-kit/tmp-dir'; import { fakeCreateCommandSurface } from '../../../__tests__/support/command-surface-fake.ts'; +import { withClientReplayScriptSources } from '@agent-device/replay-port/replay-script-source-fixtures'; +import { makeIosSession } from '../../../__tests__/support/session-factories.ts'; +import { + mockBindDeviceRuntime, + mockInspectDeviceRuntimeFacts, +} from '../../../__tests__/support/handlers/session-command-harness.ts'; +import type { EndedLeaseLedger } from '../../../core/request-ended-leases.ts'; test('replay test handler threads the complete video owner into the application command', async () => { const root = mkdtempForTestSync('agent-device-replay-handler-video-'); @@ -55,3 +63,63 @@ test('replay test handler threads the complete video owner into the application expect(response.error.code).toBe('INVALID_ARGS'); expect(response.error.message).not.toContain('Screen-recording runtime is not configured'); }); + +test('replay test session cleanup reports the lease it ended on the test request', async () => { + const root = mkdtempForTestSync('agent-device-replay-handler-ended-lease-'); + const scriptPath = path.join(root, 'opens.ad'); + fs.writeFileSync(scriptPath, 'context platform=ios\nopen "com.example.app"\n'); + const sessionStore = makeSessionStore(); + const leaseRegistry = new LeaseRegistry(); + const lease = leaseRegistry.allocateLease({ tenantId: 'tenant-a', runId: 'run-1' }); + const endedLeases: EndedLeaseLedger = []; + const wireReq = await withClientReplayScriptSources({ + token: 'token', + session: 'default', + command: 'test', + positionals: [scriptPath], + flags: {}, + meta: { cwd: root, requestId: 'replay-handler-ended-lease' }, + }); + const params: SessionCommandParams = { + createCommandSurface: fakeCreateCommandSurface, + req: { ...wireReq, internal: { endedLeases } }, + sessionName: 'default', + logPath: path.join(root, 'daemon.log'), + sessionStore, + platformServices: daemonPlatformServicesFixture(), + leaseRegistry, + invoke: async (nestedReq) => { + if (nestedReq.command === 'open' && nestedReq.session) { + sessionStore.publish( + nestedReq.session, + makeIosSession(nestedReq.session, { + lease: { leaseId: lease.leaseId, tenantId: lease.tenantId, runId: lease.runId }, + }), + ); + } + return { ok: true, data: {} }; + }, + inspectFacts: mockInspectDeviceRuntimeFacts, + bindDevice: mockBindDeviceRuntime, + bindExactDevice: unavailableBindExactDevice, + screenRecordingAdmissionLedger: createScreenRecordingAdmissionLedger(), + requestScope: { + signal: new AbortController().signal, + diagnostics: { emit: () => {} }, + progress: { report: () => {} }, + }, + retainDeviceExecutionLock: async () => {}, + throwIfCanceled: () => {}, + reconcileOrphanedDeviceClaim: async () => ({ + status: 'retained' as const, + reason: 'handler-test' as const, + }), + platformResourceCleanup: platformResourceCleanupFixture(), + }; + + const response = await handleReplayTestCommand(params); + + expect(response).toMatchObject({ ok: true }); + expect(endedLeases).toEqual([{ leaseId: lease.leaseId, outcome: 'released' }]); + expect(leaseRegistry.listActiveLeases()).toHaveLength(0); +}); diff --git a/packages/daemon/src/sessions/handlers/session-replay-command.ts b/packages/daemon/src/sessions/handlers/session-replay-command.ts index e2c0e022d4..12833f230a 100644 --- a/packages/daemon/src/sessions/handlers/session-replay-command.ts +++ b/packages/daemon/src/sessions/handlers/session-replay-command.ts @@ -27,6 +27,7 @@ import { resolveSessionScope } from '../session-routing.ts'; import { resolveEffectiveOpenRuntimeHints } from '../session-runtime.ts'; import { captureSnapshot } from '../../core/snapshot-capture.ts'; import { handleSessionCloseCommands } from '../session-lifecycle/index.ts'; +import { onDelegatingLedger } from '../../core/request-ended-leases.ts'; import { createReplayTestVideoOwner } from './session-replay-video-owner.ts'; import type { SessionCommandHandler } from './session-command-input.ts'; @@ -191,14 +192,17 @@ async function closeReplayTestSession(params: ReplayTestSessionCleanupParams): P const { req, sessionName, logPath, sessionStore } = params; if (!sessionStore.get(sessionName)) return; const closeResponse = await handleSessionCloseCommands({ - req: { - token: req.token, - session: sessionName, - command: 'close', - positionals: [], - flags: {}, - meta: req.meta, - }, + req: onDelegatingLedger( + { + token: req.token, + session: sessionName, + command: 'close', + positionals: [], + flags: {}, + meta: req.meta, + }, + req, + ), sessionName, logPath, sessionStore, diff --git a/packages/daemon/src/sessions/lease-lifecycle.ts b/packages/daemon/src/sessions/lease-lifecycle.ts index b6f2bc421c..91d734e336 100644 --- a/packages/daemon/src/sessions/lease-lifecycle.ts +++ b/packages/daemon/src/sessions/lease-lifecycle.ts @@ -152,14 +152,17 @@ export function resolveSessionLeaseForRequest(params: { ); } -/** Releases the lease a closing session holds, unless its owner allocated it with `retainOnClose`. */ +/** + * Releases the lease a closing session holds, unless its owner allocated it with `retainOnClose`. + * `endedLeaseId` names the lease the close ended, including one that was already inactive. + */ export async function releaseSessionLease(params: { session: SessionState; leaseRegistry: LeaseRegistry; leaseLifecycleProvider?: LeaseLifecycleProvider; -}): Promise | undefined> { +}): Promise<{ endedLeaseId?: string; providerData?: Record }> { const lease = params.session.lease; - if (!lease) return undefined; + if (!lease) return {}; const releaseRequest = leaseScopeToReleaseRequest({ leaseId: lease.leaseId, tenantId: lease.tenantId, @@ -181,7 +184,7 @@ export async function releaseSessionLease(params: { retained: true, }, }); - return undefined; + return {}; } const providerData = activeLease ? await params.leaseLifecycleProvider?.release?.(activeLease) @@ -200,5 +203,5 @@ export async function releaseSessionLease(params: { released: result.released, }, }); - return providerData; + return { endedLeaseId: lease.leaseId, ...(providerData ? { providerData } : {}) }; } diff --git a/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-close-ended-lease.test.ts b/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-close-ended-lease.test.ts new file mode 100644 index 0000000000..b10e1ab0a7 --- /dev/null +++ b/packages/daemon/src/sessions/session-lifecycle/internal/__tests__/session-close-ended-lease.test.ts @@ -0,0 +1,147 @@ +import { daemonPlatformServicesFixture } from '../../../../__tests__/support/platform-services-fixture.ts'; +import { beforeEach, expect, test, vi } from 'vitest'; +import path from 'node:path'; +import { AppError } from '@agent-device/kernel/errors'; +import { mkdtempForTestSync } from '@agent-device/test-kit/tmp-dir'; +import { SessionStore } from '../../../../core/session-store.ts'; +import type { DaemonRequest, DaemonResponse } from '../../../../core/daemon-request.ts'; +import type { SessionState } from '../../../../core/session-state.ts'; +import { LeaseRegistry } from '../../../../resources/lease-registry.ts'; +import { fakeCreateCommandSurface } from '../../../../__tests__/support/command-surface-fake.ts'; +import { handleSessionCommands } from '../../../../__tests__/support/handlers/session-command-harness.ts'; +import { dispatchApplicationLifecycleEffect } from '../../../../__tests__/support/application-lifecycle-runtime-fixture.ts'; +import type { EndedLeaseLedger } from '../../../../core/request-ended-leases.ts'; + +const mockDispatch = vi.mocked(dispatchApplicationLifecycleEffect); +const noopInvoke = async (_req: DaemonRequest): Promise => ({ ok: true, data: {} }); +const SESSION_NAME = 'close-ended-lease'; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +function leasedSession(leaseRegistry: LeaseRegistry, retainOnClose?: true): SessionState { + const lease = leaseRegistry.allocateLease({ + tenantId: 'tenant-a', + runId: 'run-1', + leaseBackend: 'android-instance', + leaseProvider: 'proxy', + deviceKey: 'android:emulator-5554', + clientId: 'client-a', + ...(retainOnClose ? { retainOnClose } : {}), + }); + return { + ...unleasedSession(), + lease: { + leaseId: lease.leaseId, + tenantId: lease.tenantId, + runId: lease.runId, + leaseBackend: lease.backend, + leaseProvider: lease.leaseProvider, + deviceKey: lease.deviceKey, + clientId: lease.clientId, + }, + }; +} + +function unleasedSession(): SessionState { + return { + name: SESSION_NAME, + device: { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator', + target: 'mobile', + booted: true, + }, + createdAt: Date.now(), + actions: [], + }; +} + +function close( + session: SessionState, + leaseRegistry: LeaseRegistry, + positionals: string[] = [], +): { response: Promise; endedLeases: EndedLeaseLedger } { + const root = mkdtempForTestSync('agent-device-session-close-ended-lease-'); + const sessionStore = new SessionStore(path.join(root, 'sessions')); + sessionStore.publish(SESSION_NAME, session); + const endedLeases: EndedLeaseLedger = []; + const response = handleSessionCommands({ + createCommandSurface: fakeCreateCommandSurface, + platformServices: daemonPlatformServicesFixture(), + req: { + token: 't', + session: SESSION_NAME, + command: 'close', + positionals, + flags: {}, + internal: { endedLeases }, + }, + sessionName: SESSION_NAME, + logPath: path.join(root, 'daemon.log'), + sessionStore, + leaseRegistry, + invoke: noopInvoke, + }); + return { response, endedLeases }; +} + +test('close records the lease it released on the request ledger', async () => { + const leaseRegistry = new LeaseRegistry(); + const session = leasedSession(leaseRegistry); + + const { response, endedLeases } = close(session, leaseRegistry); + + expect(await response).toMatchObject({ ok: true }); + expect(endedLeases).toEqual([{ leaseId: session.lease?.leaseId, outcome: 'released' }]); + expect(leaseRegistry.listActiveLeases()).toHaveLength(0); +}); + +test('close records a lease released before its lease-end teardown reached the session', async () => { + const leaseRegistry = new LeaseRegistry(); + const session = leasedSession(leaseRegistry); + const lease = session.lease!; + const { released } = leaseRegistry.releaseLease({ + leaseId: lease.leaseId, + tenantId: lease.tenantId, + runId: lease.runId, + leaseBackend: lease.leaseBackend, + leaseProvider: lease.leaseProvider, + deviceKey: lease.deviceKey, + clientId: lease.clientId, + }); + expect(released).toBe(true); + + const { response, endedLeases } = close(session, leaseRegistry); + + expect(await response).toMatchObject({ ok: true }); + expect(endedLeases).toEqual([{ leaseId: lease.leaseId, outcome: 'released' }]); +}); + +test('close records no ended lease for a retainOnClose lease or a session without one', async () => { + const leaseRegistry = new LeaseRegistry(); + for (const session of [leasedSession(leaseRegistry, true), unleasedSession()]) { + const { response, endedLeases } = close(session, leaseRegistry); + + expect(await response).toMatchObject({ ok: true }); + expect(endedLeases).toEqual([]); + } + expect(leaseRegistry.listActiveLeases()).toHaveLength(1); +}); + +test('a close that fails after releasing its lease still records the release', async () => { + const leaseRegistry = new LeaseRegistry(); + const session = { ...leasedSession(leaseRegistry), appBundleId: 'com.example.app' }; + mockDispatch.mockRejectedValueOnce( + new AppError('DEVICE_UNAVAILABLE', 'platform close failed', { reason: 'device_disconnected' }), + ); + + const { response, endedLeases } = close(session, leaseRegistry, ['com.example.app']); + + await expect(response).rejects.toMatchObject({ code: 'DEVICE_UNAVAILABLE' }); + expect(endedLeases).toEqual([{ leaseId: session.lease?.leaseId, outcome: 'released' }]); + expect(leaseRegistry.listActiveLeases()).toHaveLength(0); +}); diff --git a/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts b/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts index c59c123a47..bf97d646b7 100644 --- a/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts +++ b/packages/daemon/src/sessions/session-lifecycle/internal/session-close.ts @@ -10,6 +10,7 @@ import { resolveCommandDevice } from '../../../resources/session-device-resoluti import { expireRefFrame } from '../../../core/ref-frame.ts'; import type { LeaseRegistry } from '../../../resources/lease-registry.ts'; import { releaseSessionLease } from '../../lease-lifecycle.ts'; +import { recordEndedLease } from '../../../core/request-ended-leases.ts'; import { hasRepairPlatformCloseReceipt, isRepairArmedSession, @@ -116,12 +117,15 @@ async function prepareRepairClose(params: { } async function releaseProviderLeaseForClose(params: { + req: DaemonRequest; session: SessionState; leaseRegistry: LeaseRegistry; leaseLifecycleProvider: LeaseLifecycleProvider | undefined; }): Promise<{ providerData?: Record; response?: DaemonResponse }> { try { - return { providerData: await releaseSessionLease(params) }; + const { endedLeaseId, providerData } = await releaseSessionLease(params); + if (endedLeaseId) recordEndedLease(params.req, endedLeaseId); + return { providerData }; } catch (error) { const normalized = normalizeError(error); return { @@ -324,6 +328,7 @@ async function runCloseTeardownAndRelease(params: { }); let session = sessionStore.requireCurrent(ref); const leaseRelease = await releaseProviderLeaseForClose({ + req, session, leaseRegistry, leaseLifecycleProvider, diff --git a/src/__tests__/cli-close.test.ts b/src/__tests__/cli-close.test.ts index 31cc99e451..9c7d88c5d4 100644 --- a/src/__tests__/cli-close.test.ts +++ b/src/__tests__/cli-close.test.ts @@ -1,7 +1,16 @@ import { test } from 'vitest'; import assert from 'node:assert/strict'; import { AppError } from '@agent-device/kernel/errors'; +import fs from 'node:fs'; +import path from 'node:path'; import { runCliCapture, type CapturedCliRun } from './cli-capture.ts'; +import { + connectionWorkspace, + seedConnectionState, + type StoredConnectionSeed, +} from './remote-connection.fixtures.ts'; +import { readRemoteConnectionState } from '../remote/remote-connection-state.ts'; +import type { DaemonRequest, DaemonResponse } from '../daemon-client/daemon-client.ts'; function runCliWithDaemonStartupFailure(argv: string[]): Promise { return runCliCapture(argv, async () => { @@ -85,3 +94,142 @@ test('close --shutdown --json treats daemon startup failure as no-op success', a assert.equal(payload.data.source, 'no-daemon'); assert.equal(result.stderr, ''); }); + +const PROXY_LEASE_ID = 'abc123abc123abc1'; +const ENDED_LEASES = [{ leaseId: PROXY_LEASE_ID, outcome: 'released' }]; + +function seedProxyConnection(overrides: Partial = {}): string { + const { stateDir, remoteConfigPath } = connectionWorkspace('agent-device-close-proxy-'); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-proxy', + remoteConfigPath, + daemon: { baseUrl: 'https://daemon.example' }, + tenant: 'proxy', + runId: 'proxy-client-1', + leaseId: PROXY_LEASE_ID, + leaseBackend: 'ios-instance', + leaseProvider: 'proxy', + clientId: 'client-1', + deviceKey: 'ios:mobile:SIM-001', + platform: 'ios', + ...overrides, + }, + }); + return stateDir; +} + +function storedLeaseId(stateDir: string): string | undefined { + return readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId; +} + +function runProxyCli( + stateDir: string, + argv: string[], + respond: (req: Omit) => DaemonResponse, +): Promise { + return runCliCapture( + [...argv, '--state-dir', stateDir, '--session', 'adc-proxy', '--json'], + async (req) => + req.command === 'lease_heartbeat' + ? { + ok: true, + data: { + lease: { + leaseId: PROXY_LEASE_ID, + tenantId: 'proxy', + runId: 'proxy-client-1', + backend: 'ios-instance', + }, + }, + } + : respond(req), + ); +} + +test('proxy close forgets the stored lease the daemon reports ended, and sends no release', async () => { + const stateDir = seedProxyConnection(); + const result = await runProxyCli(stateDir, ['close'], () => ({ + ok: true, + data: { session: 'adc-proxy', endedLeases: ENDED_LEASES }, + })); + assert.equal(result.code, null); + assert.deepEqual( + result.calls.map((call) => call.command), + ['close'], + ); + assert.equal(storedLeaseId(stateDir), undefined); + assert.equal( + readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseProvider, + 'proxy', + ); +}); + +test('a failed proxy close forgets the stored lease the daemon already ended', async () => { + const stateDir = seedProxyConnection(); + const result = await runProxyCli(stateDir, ['close'], () => ({ + ok: false, + error: { + code: 'DEVICE_UNAVAILABLE', + message: 'platform close failed', + details: { endedLeases: ENDED_LEASES }, + }, + })); + assert.equal(result.code, 1); + assert.equal(storedLeaseId(stateDir), undefined); +}); + +test('a proxy batch forgets the stored lease its reply reports ended by a close step', async () => { + const closeStep = { step: 1, command: 'close', ok: true, data: {}, durationMs: 1 }; + const batchReplies: DaemonResponse[] = [ + { + ok: true, + data: { + total: 1, + executed: 1, + totalDurationMs: 1, + results: [closeStep], + endedLeases: ENDED_LEASES, + }, + }, + { + ok: false, + error: { + code: 'SESSION_NOT_FOUND', + message: 'Batch failed at step 2 (snapshot): No active session', + details: { step: 2, partialResults: [closeStep], endedLeases: ENDED_LEASES }, + }, + }, + ]; + for (const reply of batchReplies) { + const stateDir = seedProxyConnection(); + const result = await runProxyCli( + stateDir, + ['batch', '--steps', '[{"command":"close","input":{}},{"command":"snapshot","input":{}}]'], + () => reply, + ); + assert.deepEqual( + result.calls.map((call) => call.command).filter((command) => command !== 'lease_heartbeat'), + ['batch'], + ); + assert.equal(storedLeaseId(stateDir), undefined); + } +}); + +test('a proxy replay forgets the stored lease its reply reports ended by the script close', async () => { + const stateDir = seedProxyConnection(); + const scriptPath = path.join(stateDir, 'open-close.ad'); + fs.writeFileSync(scriptPath, 'open "com.apple.Preferences"\nclose\n'); + const result = await runProxyCli(stateDir, ['replay', scriptPath], () => ({ + ok: true, + data: { replayed: 2, healed: 0, session: 'adc-proxy', endedLeases: ENDED_LEASES }, + })); + assert.equal(result.code, null); + assert.deepEqual( + result.calls.map((call) => call.command).filter((command) => command !== 'lease_heartbeat'), + ['replay'], + ); + assert.equal(storedLeaseId(stateDir), undefined); +}); diff --git a/src/cli.ts b/src/cli.ts index 3dfb3fc23d..88d9f4abff 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -811,6 +811,29 @@ function hasExplicitMetroRuntimeOverrides(explicitFlagKeys: Set): boole return false; } +type CliDaemonTransportSetup = { + command: string; + flags: CliFlags; + replayTestReporterRuntime?: ReplayTestReporterRuntime; + commandProgress: CommandProgressState; + transport: CliDaemonTransport; +}; + +/** + * A remote connection reads each daemon reply for the leases its request ended, and forgets the + * stored lease when the reply names it. + */ +function createCliDaemonTransport(options: CliDaemonTransportSetup): AgentDeviceDaemonTransport { + const send = createCliProgressTransport(options); + if (!options.flags.remoteConfig) return send; + return async (req, context) => { + const response = await send(req, context); + const { forgetEndedProxyLease } = await import('./cli/connection/ended-lease.ts'); + forgetEndedProxyLease(options.flags, response); + return response; + }; +} + /** * The human CLI renders streamed progress itself: `test` hands its events to the * replay-test reporter, every other command streams `command` lines to stderr @@ -818,13 +841,7 @@ function hasExplicitMetroRuntimeOverrides(explicitFlagKeys: Set): boole * doctor summary does not repeat checks progress already printed). `--json` opts * out of progress entirely, so it installs no sink. */ -function createCliDaemonTransport(options: { - command: string; - flags: CliFlags; - replayTestReporterRuntime?: ReplayTestReporterRuntime; - commandProgress: CommandProgressState; - transport: CliDaemonTransport; -}): AgentDeviceDaemonTransport { +function createCliProgressTransport(options: CliDaemonTransportSetup): AgentDeviceDaemonTransport { const { command, flags, replayTestReporterRuntime, transport } = options; if (flags.json) return createClientDaemonTransport(transport); const onProgress = diff --git a/src/cli/commands/connection-runtime.ts b/src/cli/commands/connection-runtime.ts index 7abb11f75b..68475522be 100644 --- a/src/cli/commands/connection-runtime.ts +++ b/src/cli/commands/connection-runtime.ts @@ -580,7 +580,7 @@ export async function stopReactDevtoolsCleanup(options: { } /** The host allocated a `macos-app` lease and ends it itself; a tenant never releases it. */ -function isHostAllocatedLease(state: RemoteConnectionState): boolean { +export function isHostAllocatedLease(state: RemoteConnectionState): boolean { return state.leaseBackend === 'macos-app'; } diff --git a/src/cli/connection/ended-lease.test.ts b/src/cli/connection/ended-lease.test.ts new file mode 100644 index 0000000000..eadfc340d2 --- /dev/null +++ b/src/cli/connection/ended-lease.test.ts @@ -0,0 +1,80 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import { test } from 'vitest'; +import type { DaemonResponse } from '@agent-device/kernel/contracts'; +import { + connectionWorkspace, + seedConnectionState, + type StoredConnectionSeed, +} from '../../__tests__/remote-connection.fixtures.ts'; +import { readRemoteConnectionState } from '../../remote/remote-connection-state.ts'; +import { forgetEndedProxyLease } from './ended-lease.ts'; + +const LEASE_ID = 'abc123abc123abc1'; +const ENDED = [{ leaseId: LEASE_ID, outcome: 'released' }] as const; + +function storedLeaseIdAfter( + response: DaemonResponse, + overrides: Partial = {}, +): string | undefined { + const { stateDir, remoteConfigPath } = connectionWorkspace('agent-device-ended-lease-'); + fs.writeFileSync(remoteConfigPath, JSON.stringify({ daemonBaseUrl: 'https://daemon.example' })); + seedConnectionState({ + stateDir, + state: { + session: 'adc-proxy', + remoteConfigPath, + tenant: 'proxy', + runId: 'proxy-client-1', + leaseId: LEASE_ID, + leaseBackend: 'ios-instance', + leaseProvider: 'proxy', + platform: 'ios', + ...overrides, + }, + }); + forgetEndedProxyLease( + { + json: true, + help: false, + version: false, + stateDir, + remoteConfig: remoteConfigPath, + session: 'adc-proxy', + }, + response, + ); + return readRemoteConnectionState({ stateDir, session: 'adc-proxy' })?.leaseId; +} + +test('an ended lease is forgotten from a reply or its error details', () => { + const replies: DaemonResponse[] = [ + { ok: true, data: { session: 'adc-proxy', endedLeases: [...ENDED] } }, + { + ok: false, + error: { code: 'COMMAND_FAILED', message: 'failed', details: { endedLeases: [...ENDED] } }, + }, + ]; + for (const reply of replies) assert.equal(storedLeaseIdAfter(reply), undefined); +}); + +test('the stored lease stays unless the reply ends that lease at response level', () => { + const nestedStep = { step: 1, command: 'close', ok: true, data: { endedLeases: [...ENDED] } }; + const replies: DaemonResponse[] = [ + { ok: true, data: { endedLeases: [{ leaseId: 'other-lease', outcome: 'released' }] } }, + { ok: true, data: { endedLeases: [{ leaseId: LEASE_ID, outcome: 'retained' }] } }, + { ok: true, data: { endedLeases: { leaseId: LEASE_ID, outcome: 'released' } } }, + { ok: true, data: { lease: { leaseId: LEASE_ID, tenantId: 'proxy', runId: 'run' } } }, + { ok: true, data: { total: 1, executed: 1, results: [nestedStep] } }, + ]; + for (const reply of replies) assert.equal(storedLeaseIdAfter(reply), LEASE_ID); +}); + +test('an ended lease the CLI does not own stays stored: host-allocated or non-proxy', () => { + const reply: DaemonResponse = { ok: true, data: { endedLeases: [...ENDED] } }; + assert.equal( + storedLeaseIdAfter(reply, { leaseBackend: 'macos-app', platform: 'macos' }), + LEASE_ID, + ); + assert.equal(storedLeaseIdAfter(reply, { leaseProvider: undefined }), LEASE_ID); +}); diff --git a/src/cli/connection/ended-lease.ts b/src/cli/connection/ended-lease.ts new file mode 100644 index 0000000000..c6e12f5ce4 --- /dev/null +++ b/src/cli/connection/ended-lease.ts @@ -0,0 +1,29 @@ +import type { DaemonResponse } from '@agent-device/kernel/contracts'; +import type { CliFlags } from '@agent-device/contracts/command'; +import { readEndedLeases } from '@agent-device/contracts/lease-scope'; +import { resolveDaemonPaths } from '@agent-device/daemon-contracts/daemon-resolution'; +import { + readRemoteConnectionState, + writeRemoteConnectionState, +} from '../../remote/remote-connection-state.ts'; +import { isHostAllocatedLease } from '../commands/connection-runtime.ts'; +import { connectionProviderCapabilities } from './provider-policy.ts'; + +/** + * Forgets the stored proxy lease once a daemon reply says its request ended that lease, so the next + * `open` allocates instead of heartbeating a lease that no longer exists. + */ +export function forgetEndedProxyLease(flags: CliFlags, response: DaemonResponse): void { + if (!flags.remoteConfig) return; + const endedLeaseIds = readEndedLeases(response).map((lease) => lease.leaseId); + if (endedLeaseIds.length === 0) return; + const stateDir = resolveDaemonPaths(flags.stateDir).baseDir; + const state = readRemoteConnectionState({ stateDir, session: flags.session ?? 'default' }); + if (!state?.leaseId || !endedLeaseIds.includes(state.leaseId)) return; + if (isHostAllocatedLease(state)) return; + if (connectionProviderCapabilities(state.leaseProvider).leaseKind !== 'proxy') return; + writeRemoteConnectionState({ + stateDir, + state: { ...state, leaseId: undefined, updatedAt: new Date().toISOString() }, + }); +} diff --git a/website/docs/docs/remote-proxy.md b/website/docs/docs/remote-proxy.md index c123cd6c62..add7878de9 100644 --- a/website/docs/docs/remote-proxy.md +++ b/website/docs/docs/remote-proxy.md @@ -45,7 +45,7 @@ agent-device disconnect `--daemon-auth-token ` works instead of the environment variable, but it authenticates only the command you pass it to. Later commands need the token again through the environment variable, a `daemonAuthToken` entry in your remote config profile, or the flag on each command. -`connect proxy` stores the proxy profile and client identity. `open` leases the device for you, and the lease expires after five minutes without commands. A lease allocated directly over the RPC without `ttlMs` gets the daemon's one-minute inactivity default instead. Either window starts when allocation completes, not when it was requested. `close` releases the active session and its device lease, unless the lease was allocated with `retainOnClose`; only `leases.release`, expiry, or daemon shutdown ends that one. `disconnect` clears local connection state. +`connect proxy` stores the proxy profile and client identity. `open` leases the device for you, and the lease expires after five minutes without commands. A lease allocated directly over the RPC without `ttlMs` gets the daemon's one-minute inactivity default instead. Either window starts when allocation completes, not when it was requested. `close` releases the active session and its device lease, unless the lease was allocated with `retainOnClose`; only `leases.release`, expiry, or daemon shutdown ends that one. The reply to any request that closed a session lists the leases it ended in `endedLeases`, as `{ leaseId, outcome: 'released' }`. This covers `close`, a `batch` with a close step, and a `replay` or `test` script that closes, as well as the session cleanup `test` runs after each script. A successful reply has the field in `data`, and a failed one has it in `error.details`. A gateway in front of the daemon reads the lease end from that reply. When the reply names the stored lease, the CLI forgets it, so the next `open` leases a device again. `disconnect` clears local connection state. `open` names each new lease with an id it makes (32 lowercase hex characters) and sends it with the allocation, so a host in front of the daemon knows the lease id before the daemon allocates. A direct `agent_device.lease.allocate` RPC can pass `leaseId` (16 to 128 hex characters) the same way; an id this daemon has already given a lease, active or ended, is refused with `INVALID_ARGS` and reason `LEASE_ID_IN_USE`, and without one the daemon makes the id. Use the id the reply carries: a run that already holds a lease gets that lease back, and an older daemon ignores the requested id.