diff --git a/packages/provider-revyl/src/lease-owner.test.ts b/packages/provider-revyl/src/lease-owner.test.ts new file mode 100644 index 0000000000..25dce5abab --- /dev/null +++ b/packages/provider-revyl/src/lease-owner.test.ts @@ -0,0 +1,223 @@ +import assert from 'node:assert/strict'; +import { afterEach, test, vi } from 'vitest'; +import { AppError } from '@agent-device/kernel/errors'; +import { buildRevylDevice } from './device.ts'; +import { createRevylRuntime } from './runtime.ts'; +import { REVYL_STOP_REQUEST_BUDGET_MS } from './revyl-client.ts'; +import { revylLease } from './revyl-backend.fixtures.ts'; + +const device = buildRevylDevice('ios', revylLease('ios'), 'run-1'); +const lease = revylLease('ios'); +const startPath = '/api/v1/execution/start_device'; +const stopPath = '/api/v1/execution/device/sessions/'; + +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((report) => { + resolve = report; + }); + return { promise, resolve }; +} + +function harness() { + const start = deferred(); + const stop = deferred(); + let creates = 0; + let stops = 0; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: URL | string) => { + const path = new URL(input).pathname; + if (path === startPath) { + creates++; + return await start.promise; + } + if (path.startsWith(stopPath)) { + stops++; + return await stop.promise; + } + if (path.includes('worker-connection')) + return Response.json({ status: 'ready', worker_ws_url: 'wss://test' }); + return Response.json({ device_connected: true }); + }), + ); + const { runtime } = createRevylRuntime({ apiKey: 'test-key', clientVersion: 'test' }); + return { runtime, start, stop, creates: () => creates, stops: () => stops }; +} +const started = () => Response.json({ workflow_run_id: 'run-1' }); +const stopped = () => Response.json({ success: true, request_accepted: true }); +const retired = (error: unknown) => + error instanceof AppError && error.details?.reason === 'provider-lease-releasing'; + +afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); + +test('release immediately retires interactors, inventory and repeat allocation, including after failure', async () => { + const h = harness(); + h.start.resolve(started()); + await h.runtime.leaseLifecycle!.allocate!(lease); + assert.ok(h.runtime.getInteractor(device)); + const release = h.runtime.leaseLifecycle!.release!(lease); + const rejected = release.then( + () => undefined, + (error: unknown) => error, + ); + try { + assert.equal(h.runtime.hasLiveSession(device), false); + assert.equal(h.runtime.getInteractor(device), undefined); + assert.equal( + await h.runtime.deviceInventoryProvider!({ leaseProvider: 'revyl', leaseId: lease.leaseId }), + null, + ); + await assert.rejects(h.runtime.leaseLifecycle!.allocate!(lease), retired); + } finally { + h.stop.resolve(Response.json({ success: false })); + await rejected; + } + const error = await rejected; + assert.ok(error instanceof AppError); + assert.equal(error.details?.cleanup, 'uncertain'); + assert.equal(h.runtime.getInteractor(device), undefined); + await assert.rejects(h.runtime.leaseLifecycle!.allocate!(lease), retired); + assert.equal(h.creates(), 1); +}); + +test('release before the first allocation await refuses the device it takes', async () => { + const h = harness(); + const allocation = h.runtime.leaseLifecycle!.allocate!(lease); + const refused = allocation.then(() => false, retired); + const disposal = h.runtime.leaseLifecycle!.release!(lease); + h.start.resolve(started()); + h.stop.resolve(stopped()); + await disposal; + assert.equal(h.stops(), 1); + assert.equal(await refused, true); + assert.equal(h.runtime.hasLiveSession(device), false); +}); + +test('a caller that gives up during release retains the typed teardown reason', async () => { + const h = harness(); + const controller = new AbortController(); + const allocation = h.runtime.leaseLifecycle!.allocate!(lease, { signal: controller.signal }); + await vi.waitFor(() => assert.equal(h.creates(), 1)); + const disposal = h.runtime.leaseLifecycle!.release!(lease); + controller.abort(); + try { + await assert.rejects( + allocation, + (error: unknown) => + error instanceof AppError && + error.details?.reason === 'provider-lease-releasing' && + error.cause instanceof AppError && + error.cause.details?.reason === 'request_canceled', + ); + } finally { + h.start.resolve(started()); + h.stop.resolve(stopped()); + await disposal; + } + assert.equal(h.stops(), 1); +}); + +test.each(['release', 'shutdown'] as const)( + '%s bounds a pending create and ends its late answer exactly once', + async (by) => { + const h = harness(); + const allocation = h.runtime.leaseLifecycle!.allocate!(lease); + const refused = allocation.then( + () => false, + (error: unknown) => error instanceof AppError, + ); + await vi.waitFor(() => assert.equal(h.creates(), 1)); + vi.useFakeTimers(); + const disposal = + by === 'release' ? h.runtime.leaseLifecycle!.release!(lease) : h.runtime.shutdown(); + let settled = false; + const observed = disposal + .then( + () => undefined, + (error: unknown) => error, + ) + .finally(() => { + settled = true; + }); + // Cross the 120-second disposal wait without settling the transport's create promise. + await vi.advanceTimersByTimeAsync(125_000); + try { + assert.equal(settled, true, 'disposal must answer within its creation budget'); + const error = await observed; + assert.ok(error instanceof AppError); + assert.equal(error.details?.cleanup, 'uncertain'); + } finally { + h.start.resolve(started()); + h.stop.resolve(stopped()); + await observed; + await refused; + } + assert.equal(await refused, true); + await vi.waitFor(() => assert.equal(h.stops(), 1)); + await h.runtime.shutdown(); + assert.equal(h.stops(), 1); + assert.equal(h.creates(), 1); + }, +); + +test('release waits beyond the HTTP attempt deadline for the stop result', async () => { + const h = harness(); + h.start.resolve(started()); + await h.runtime.leaseLifecycle!.allocate!(lease); + vi.useFakeTimers(); + let settled = false; + const release = h.runtime.leaseLifecycle!.release!(lease).then( + (value) => { + settled = true; + return value; + }, + (error: unknown) => { + settled = true; + return error; + }, + ); + // The HTTP attempt owns 30 seconds before its own timeout can settle the request. + await vi.advanceTimersByTimeAsync(30_001); + try { + assert.equal(settled, false, 'disposal must not race the HTTP attempt deadline'); + } finally { + h.stop.resolve(stopped()); + await release; + } + assert.equal((await release) instanceof AppError, false); + assert.equal(h.stops(), 1); +}); + +test('an unanswered stop reports uncertainty, joins the pending stop, and accepts its late confirmation', async () => { + const h = harness(); + h.start.resolve(started()); + await h.runtime.leaseLifecycle!.allocate!(lease); + vi.useFakeTimers(); + let settled = false; + const release = h.runtime.leaseLifecycle!.release!(lease); + const observed = release + .then( + () => undefined, + (error: unknown) => error, + ) + .finally(() => { + settled = true; + }); + // Cross the HTTP retry budget and the disposal's 5 s settlement margin. + await vi.advanceTimersByTimeAsync(REVYL_STOP_REQUEST_BUDGET_MS + 10_000); + try { + assert.equal(settled, true, 'release must bound an unanswered stop'); + const error = await observed; + assert.ok(error instanceof AppError); + assert.equal(error.details?.cleanup, 'uncertain'); + } finally { + h.stop.resolve(stopped()); + await observed; + } + await h.runtime.leaseLifecycle!.release!(lease); + assert.equal(h.stops(), 1); +}); diff --git a/packages/provider-revyl/src/lease-owner.ts b/packages/provider-revyl/src/lease-owner.ts new file mode 100644 index 0000000000..71ecb57d14 --- /dev/null +++ b/packages/provider-revyl/src/lease-owner.ts @@ -0,0 +1,242 @@ +import type { DeviceLease, LeaseLifecycleContext } from '@agent-device/contracts/device'; +import { errorMessage } from '@agent-device/kernel/errors'; +import type { RevylPlatform } from './device.ts'; +import { + isRevylSessionGone, + REVYL_STOP_REQUEST_BUDGET_MS, + type RevylClient, +} from './revyl-client.ts'; +import { + startRevylSession, + stopRevylSession, + type RevylRentedSession, + type RevylSession, + type RevylStart, +} from './revyl-session.ts'; +import type { RevylStartRequest } from './session-allocation.ts'; +import { revylSessionIdForLease } from './session-identity.ts'; + +const CREATE_ANSWER_BUDGET_MS = 120_000; +// Allow the HTTP stop's complete request/retry budget plus a 5 s settlement margin. +const STOP_ANSWER_BUDGET_MS = REVYL_STOP_REQUEST_BUDGET_MS + 5_000; + +export type RevylStopOutcome = Readonly<{ + session: RevylRentedSession; + data?: Record; +}> & + (Readonly<{ cleanup: 'confirmed' }> | Readonly<{ cleanup: 'uncertain'; failure: string }>); + +type RevylUncertainStop = Extract; +type RevylConfirmedStop = Extract; + +type StopWork = Readonly<{ pending: Promise }>; +type RevylLeaseState = + | { phase: 'starting'; rented?: RevylRentedSession } + | { phase: 'ready'; session: RevylSession } + | { + phase: 'stopping'; + disposal: Promise; + rented?: RevylRentedSession; + stop?: StopWork; + } + | { phase: 'stop-failed'; outcome: RevylUncertainStop; stop?: StopWork } + | { phase: 'closed'; outcome?: RevylConfirmedStop }; + +export class RevylLeaseOwner { + private state: RevylLeaseState = { phase: 'starting' }; + readonly creation: Promise; + private readonly client: RevylClient; + readonly lease: DeviceLease; + readonly request: RevylStartRequest; + private readonly closed: () => void; + + constructor( + client: RevylClient, + lease: DeviceLease, + platform: RevylPlatform, + request: RevylStartRequest, + context: LeaseLifecycleContext | undefined, + closed: () => void, + ) { + this.client = client; + this.lease = lease; + this.request = request; + this.closed = closed; + // The microtask starts only after the runtime publishes this owner in its map. + this.creation = Promise.resolve().then(async () => { + let result: RevylStart; + try { + result = await startRevylSession( + client, + lease, + { + platform, + appUrl: request.appUrl, + deviceModel: request.deviceModel, + osVersion: request.osVersion, + ...(context?.signal === undefined ? {} : { signal: context.signal }), + ...(context?.deadline === undefined ? {} : { deadline: context.deadline }), + }, + (rented) => this.recordRent(rented), + ); + } catch (error) { + result = { + status: 'failed', + error, + rented: { sessionId: revylSessionIdForLease(lease.leaseId) }, + }; + } + if (this.state.phase === 'starting') { + if (result.status === 'ready') this.state = { phase: 'ready', session: result.session }; + else if (result.rented === undefined) this.close(); + else this.recordRent(result.rented); + } + return result; + }); + void this.creation + .then(async (result) => { + if (result.status === 'failed' && result.rented !== undefined) await this.endFailedStart(); + }) + .catch(() => undefined); + } + + get session(): RevylSession | undefined { + return this.state.phase === 'ready' ? this.state.session : undefined; + } + + get releasing(): boolean { + return this.state.phase !== 'starting' && this.state.phase !== 'ready'; + } + + private recordRent(rented: RevylRentedSession): void { + const state = this.state; + if (state.phase === 'starting' || state.phase === 'stopping') state.rented = rented; + else if (state.phase === 'stop-failed') + this.state = { ...state, outcome: { ...state.outcome, session: rented } }; + } + + endFailedStart(): Promise { + return this.state.phase === 'stop-failed' ? Promise.resolve(this.state.outcome) : this.end(); + } + + end(): Promise { + const previous = this.state; + if (previous.phase === 'closed') return Promise.resolve(previous.outcome); + if (previous.phase === 'stopping') return previous.disposal; + const stop = previous.phase === 'stop-failed' ? previous.stop : undefined; + const rented = + previous.phase === 'starting' + ? previous.rented + : previous.phase === 'ready' + ? previous.session + : previous.outcome.session; + const disposal = Promise.resolve().then(async () => await this.dispose()); + this.state = { + phase: 'stopping', + disposal, + ...(rented === undefined ? {} : { rented }), + ...(stop === undefined ? {} : { stop }), + }; + return disposal; + } + + private async dispose(): Promise { + const created = await withinBudget(this.creation, CREATE_ANSWER_BUDGET_MS); + if (created === undefined) { + const outcome = this.uncertain('Revyl start has not answered within its disposal budget.'); + this.state = { phase: 'stop-failed', outcome }; + // No stop can confirm absence before the create answers. Its eventual answer re-enters the same owner. + void this.creation.then(async () => await this.end()).catch(() => undefined); + return outcome; + } + const target = + created.status === 'ready' + ? { runId: created.session.runId, sessionId: created.session.sessionId } + : created.rented; + if (target === undefined) { + this.close(); + return undefined; + } + const data = created.status === 'ready' ? allocationResult(created.session) : undefined; + const state = this.state; + if (state.phase !== 'stopping') throw new TypeError('Revyl disposal lost its owner'); + const pending = state.stop?.pending ?? this.stop(target, data); + const stop = { pending }; + state.stop = stop; + const outcome = await withinBudget(pending, STOP_ANSWER_BUDGET_MS); + if (outcome === undefined) { + const uncertain = { + ...this.uncertain('Revyl stop has not answered within its disposal budget.'), + ...(data === undefined ? {} : { data }), + }; + this.state = { phase: 'stop-failed', outcome: uncertain, stop }; + // A late confirmation closes the owner; a late refusal leaves the same session available for retry. + void pending + .then((late) => { + if (this.state.phase === 'stop-failed' && this.state.stop === stop) this.finish(late); + }) + .catch(() => undefined); + return uncertain; + } + this.finish(outcome); + return outcome; + } + + private async stop( + target: RevylRentedSession, + data?: Record, + ): Promise { + const result = { session: target, ...(data === undefined ? {} : { data }) }; + try { + await stopRevylSession(this.client, target.sessionId); + return { ...result, cleanup: 'confirmed' }; + } catch (error) { + return isRevylSessionGone(error) + ? { ...result, cleanup: 'confirmed' } + : { ...result, cleanup: 'uncertain', failure: errorMessage(error) }; + } + } + + private uncertain(failure: string): RevylUncertainStop { + const state = this.state; + const target = state.phase === 'stopping' ? state.rented : undefined; + return { + cleanup: 'uncertain', + session: target ?? { sessionId: revylSessionIdForLease(this.lease.leaseId) }, + failure, + }; + } + + private finish(outcome: RevylStopOutcome): void { + if (outcome.cleanup === 'confirmed') this.close(outcome); + else this.state = { phase: 'stop-failed', outcome }; + } + + private close(outcome?: RevylConfirmedStop): void { + this.state = { phase: 'closed', ...(outcome === undefined ? {} : { outcome }) }; + this.closed(); + } +} + +async function withinBudget(work: Promise, budgetMs: number): Promise { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + work, + new Promise((resolve) => { + timer = setTimeout(() => resolve(undefined), budgetMs); + timer.unref(); + }), + ]); + } finally { + if (timer !== undefined) clearTimeout(timer); + } +} + +export function allocationResult(session: RevylSession): Record { + return { + revylRunId: session.runId, + providerSessionId: session.sessionId, + device: session.device, + }; +} diff --git a/packages/provider-revyl/src/revyl-client.ts b/packages/provider-revyl/src/revyl-client.ts index 5b75f6b4e7..393a565727 100644 --- a/packages/provider-revyl/src/revyl-client.ts +++ b/packages/provider-revyl/src/revyl-client.ts @@ -23,6 +23,9 @@ const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; /** How long `start_device` may take to answer: Revyl assigns a worker and boots the device in it. */ const REVYL_START_DEVICE_TIMEOUT_MS = 120_000; const RETRY_DELAYS_MS = [500, 1_500] as const; +export const REVYL_STOP_REQUEST_BUDGET_MS = + DEFAULT_REQUEST_TIMEOUT_MS * (RETRY_DELAYS_MS.length + 1) + + RETRY_DELAYS_MS.reduce((total, delay) => total + delay, 0); /** Device-proxy actions Revyl answers to a GET; every other action is a POST. */ const DEVICE_PROXY_GET_ACTIONS = new Set(['screenshot', 'health', 'device_info', 'hierarchy']); diff --git a/packages/provider-revyl/src/revyl-session.ts b/packages/provider-revyl/src/revyl-session.ts index 76792f359e..1fab97f09e 100644 --- a/packages/provider-revyl/src/revyl-session.ts +++ b/packages/provider-revyl/src/revyl-session.ts @@ -100,6 +100,7 @@ export async function startRevylSession( client: RevylClient, lease: DeviceLease, input: RevylStartRequestInput, + recordRent: (rented: RevylRentedSession) => void, ): Promise { // The runtime issues the session id rather than reading it back, so a session this lease started // can be ended from the lease alone — including from a process that never started it. @@ -121,6 +122,8 @@ export async function startRevylSession( } const runId = typeof started.workflow_run_id === 'string' ? started.workflow_run_id : undefined; const echoed = typeof started.session_id === 'string' ? started.session_id : undefined; + const rentedSessionId = echoed ?? sessionId; + recordRent({ sessionId: rentedSessionId, runId }); if (runId && echoed && echoed !== sessionId) { // Every later stop addresses the session by the id the runtime issued, and an expired-lease // recovery in another process derives that same id from the lease. A backend that kept its own @@ -146,7 +149,7 @@ export async function startRevylSession( // when it kept its own, so the end addresses the session the backend actually holds. return { status: 'failed', - rented: { sessionId: echoed ?? sessionId }, + rented: { sessionId: rentedSessionId }, error: new AppError( 'COMMAND_FAILED', `Revyl did not start a ${input.platform} device: ${started.error ?? started.message ?? 'no workflow run was returned'}`, diff --git a/packages/provider-revyl/src/runtime.test.ts b/packages/provider-revyl/src/runtime.test.ts index fb8ce2047f..96fe1fa608 100644 --- a/packages/provider-revyl/src/runtime.test.ts +++ b/packages/provider-revyl/src/runtime.test.ts @@ -209,15 +209,18 @@ test('a release that lands while the shared start is still running stops the dev (error: unknown) => error instanceof AppError && error.details?.reason === 'request_canceled', ); + const refused = assert.rejects( + first!, + (error: unknown) => + error instanceof AppError && error.details?.reason === 'provider-lease-releasing', + ); const released = await runtime.leaseLifecycle!.release!(lease); assert.equal(released?.providerSessionId, revylSessionIdForLease('lease-1')); assert.deepEqual(stops(fixture), [ `POST /api/v1/execution/device/sessions/${revylSessionIdForLease('lease-1')}/stop`, ]); - // The caller that rented the device is not made to carry the joiner's release: it still gets its - // answer, and the runtime holds no session a later shutdown would stop twice. - assert.equal((await first)?.providerSessionId, revylSessionIdForLease('lease-1')); + await refused; await runtime.shutdown(); assert.equal(stops(fixture).length, 1); }); @@ -405,9 +408,14 @@ test('a release that lands mid-start stops the session only once Revyl has made // The release lands while the start is in flight. It waits for the answer rather than stopping the id it // only has from the lease, and that is what makes the stop below land after the session exists. + const refused = assert.rejects( + allocating!, + (error: unknown) => + error instanceof AppError && error.details?.reason === 'provider-lease-releasing', + ); const released = await runtime.leaseLifecycle!.release!(lease); assert.equal(released?.providerSessionId, revylSessionIdForLease('lease-1')); - assert.equal((await allocating)?.providerSessionId, revylSessionIdForLease('lease-1')); + await refused; const answered = fixture.answeredPaths(); const stopAt = answered.findIndex((path) => path.endsWith('/stop')); diff --git a/packages/provider-revyl/src/runtime.ts b/packages/provider-revyl/src/runtime.ts index 476a8a6119..c68193b024 100644 --- a/packages/provider-revyl/src/runtime.ts +++ b/packages/provider-revyl/src/runtime.ts @@ -26,7 +26,6 @@ import { revylCallerGaveUp, revylCallerWait, revylCleanupPending, - startRevylSession, stopRevylSession, type RevylCallerBudget, type RevylRentedSession, @@ -37,7 +36,13 @@ import { createRevylInteractor } from './interactor.ts'; import { providerRuntimeOwner } from '@agent-device/contracts/platform-runtime'; import { resolveRevylRuntimeInstance } from './runtime-instance.ts'; import { revylSessionIdForLease } from './session-identity.ts'; -import type { RevylStartRequest } from './session-allocation.ts'; +import { + rejectRefusedRevylProfileFields, + requireSameRevylStartRequest, + resolveRevylStartRequest, + type RevylStartRequest, +} from './session-allocation.ts'; +import { RevylLeaseOwner, allocationResult, type RevylStopOutcome } from './lease-owner.ts'; export type RevylRuntimeOptions = Readonly<{ apiKey: string; @@ -63,62 +68,6 @@ export type RevylRuntime = ProviderDeviceRuntime & { hasLiveSession(device: DeviceInfo): boolean; }; -/** What one lease's start ended with, as the entry that owns its cleanup records it. */ -type RevylStartOutcome = - | Readonly<{ status: 'ready'; session: RevylSession }> - | Readonly<{ status: 'failed'; error: unknown }>; - -/** What ending one lease amounted to: the session Revyl was told to end, and what it left owed. */ -type RevylStopOutcome = Readonly<{ - session: RevylRentedSession; - /** The ended session's own report, so a release answers with the device it let go of. */ - data?: Record; - /** Why Revyl did not confirm the stop; absent once the device is no longer rented. */ - failure?: string; -}>; - -/** - * One lease's place in {@link RevylRuntimeImplementation.leases}: the start that rents its device, the - * session that start produced, and the `/stop` that has not been confirmed yet. - */ -class RevylLeaseEntry { - /** What the start was asked for, which a joining allocation has to ask for too. */ - readonly request: RevylStartRequest; - /** - * Settles once this lease's `start_device` has answered and the stop that a failed start owed has - * settled with it, so a reader that awaits it cannot miss a rented device. - */ - readonly start: Promise; - /** The live device, while this lease has one. */ - session: RevylSession | undefined; - /** The session this start left under rent; absent while the request has not answered yet. */ - stop: RevylRentedSession | undefined; - /** Whether Revyl confirmed `/stop` for {@link stop}. Only this lets the entry leave the map. */ - stopConfirmed: boolean; - /** - * The `/stop` in flight for {@link stop}, shared by every release, recovery and shutdown that arrives - * while it runs, so Revyl sees one stop per attempt. Cleared once it settles, so a failed stop is - * retried by the next caller. - */ - stopping: Promise | undefined; - - /** - * `start` fills this entry in as it goes; it may write the entry's own fields, but it must never wait - * on them, because the assignment below has not happened while its body first runs. - */ - constructor( - request: RevylStartRequest, - start: (entry: RevylLeaseEntry) => Promise, - ) { - this.request = request; - this.session = undefined; - this.stop = undefined; - this.stopConfirmed = false; - this.stopping = undefined; - this.start = start(this); - } -} - /** * Builds the Revyl runtime and the owner the host loads its bindings through. The owner's module is * loaded on first bind, so a session that only rents a device never evaluates the platform-runtime @@ -147,17 +96,7 @@ export function createRevylRuntime(options: RevylRuntimeOptions): RevylRuntimeRe class RevylRuntimeImplementation implements ProviderDeviceRuntime { private readonly client: RevylClient; private readonly options: RevylRuntimeOptions; - /** - * A lease entry exists from the moment `start_device` is sent until Revyl confirms `/stop` for every - * session that start produced. Only a confirmed stop removes it. Allocate, join, release, recovery and - * shutdown all read and write this one entry; nothing else holds a cleanup handle. - * - * `start` is how every reader learns what the lease rented, including a start whose caller stopped - * waiting for it: the request outlives that caller to learn what it rented, and until it answers, - * nobody knows whether there is a device to stop. `stop` is what the entry owes, so an entry that is - * still here is an unconfirmed release, and one that rented nothing is not here at all. - */ - private readonly leases = new Map(); + private readonly leases = new Map(); private shuttingDown = false; readonly provider = REVYL_PROVIDER; @@ -172,12 +111,14 @@ class RevylRuntimeImplementation implements ProviderDeviceRuntime { // The daemon restarted and holds a persisted lease with no entry beside it. The id derived from the // lease is the only handle left, and Revyl answers a session it never had as gone. const derived = { sessionId: revylSessionIdForLease(lease.leaseId) }; - const outcome = entry - ? await this.endLease(lease.leaseId, entry) - : { session: derived, failure: await this.stopRented(derived) }; - // The daemon forgets a pending release once recovery resolves, so a stop Revyl did not confirm has to - // keep the lease pending rather than report a device Revyl may still be holding. - if (outcome?.failure !== undefined) throw revylReleasePending(lease.leaseId, outcome); + if (entry) { + const outcome = await entry.end(); + if (outcome?.cleanup === 'uncertain') throw revylReleasePending(lease.leaseId, outcome); + return; + } + const failure = await this.stopRented(derived); + if (failure !== undefined) + throw revylReleasePending(lease.leaseId, { session: derived, cleanup: 'uncertain', failure }); }; readonly deviceInventoryProvider: DeviceInventoryProvider = async (request) => { @@ -228,14 +169,12 @@ class RevylRuntimeImplementation implements ProviderDeviceRuntime { // Snapshot first: `allocate` refuses once the flag is set, and it does that before it sends anything, // so no entry can be added while this runs. const entries = [...this.leases.entries()]; - const outcomes = await Promise.all( - entries.map(async ([leaseId, entry]) => await this.endLease(leaseId, entry)), - ); + const outcomes = await Promise.all(entries.map(async ([, entry]) => await entry.end())); const unconfirmed = outcomes.flatMap((outcome) => - outcome?.failure === undefined ? [] : [{ ...outcome.session, failure: outcome.failure }], + outcome?.cleanup !== 'uncertain' ? [] : [{ ...outcome.session, failure: outcome.failure }], ); if (unconfirmed.length === 0) return; - throw revylCleanupPending(unconfirmed, { operation: 'shutdown' }); + throw revylCleanupPending(unconfirmed, { operation: 'shutdown', cleanup: 'uncertain' }); } /** @@ -256,11 +195,6 @@ class RevylRuntimeImplementation implements ProviderDeviceRuntime { ? platformForRevylLeaseBackend(lease.backend) : undefined; if (platform === undefined) return undefined; - const { - rejectRefusedRevylProfileFields, - requireSameRevylStartRequest, - resolveRevylStartRequest, - } = await import('./session-allocation.ts'); // Before the join below: an allocation that reuses a live lease still carries flags of its own, // and Revyl reads none of them — whether it starts a device or joins one that is starting. rejectRefusedRevylProfileFields(context); @@ -275,95 +209,41 @@ class RevylRuntimeImplementation implements ProviderDeviceRuntime { // The caller that started the device waits on it the same way a joiner does: one entry, one awaitable, // each caller bounded by its own patience. const entry = existing ?? this.rent(lease, platform, request, context); - const outcome = await this.join(lease.leaseId, entry, platform, context); + if (entry.releasing) throw releasingLease(lease.leaseId); + const live = entry.session; + const outcome = + live === undefined + ? await this.join(lease.leaseId, entry, platform, context) + : { status: 'ready' as const, session: live }; if (this.shuttingDown) { // The device this start rented is already an entry shutdown is ending, so this caller only reports // the refusal rather than handing a session to a runtime that is gone. throw shuttingDownDuring(lease.leaseId, 'allocate'); } - if (outcome.status === 'failed') throw outcome.error; - return allocationResult(outcome.session); + if (outcome.status === 'failed') { + const stopped = await entry.endFailedStart(); + if (stopped?.cleanup === 'uncertain') + throw revylReleasePending(lease.leaseId, stopped, outcome.error); + throw outcome.error; + } + const session = entry.session; + if (session === undefined) throw releasingLease(lease.leaseId); + return allocationResult(session); } - /** - * Rents the lease's device and returns the entry that owns what the request rents. The entry is in the - * map before the first `await`, so an allocation that arrives mid-start joins it instead of renting a - * second device, and a release that arrives then has somewhere to find the session. - */ private rent( lease: DeviceLease, platform: RevylPlatform, request: RevylStartRequest, context?: LeaseLifecycleContext, - ) { - const entry = new RevylLeaseEntry(request, (target) => - this.runStart(target, lease, platform, context), - ); + ): RevylLeaseOwner { + const entry = new RevylLeaseOwner(this.client, lease, platform, request, context, () => { + if (this.leases.get(lease.leaseId) === entry) this.leases.delete(lease.leaseId); + }); this.leases.set(lease.leaseId, entry); return entry; } - /** - * Starts the lease's device, records what it rented in its entry, and ends from that entry a start that - * never became ready — before the failure reaches the caller, so a failed allocate never leaves a device - * running with only a later release to end it. A start Revyl refused outright rented nothing, so its - * entry is closed rather than kept. - */ - private async runStart( - entry: RevylLeaseEntry, - lease: DeviceLease, - platform: RevylPlatform, - context?: LeaseLifecycleContext, - ): Promise { - const leaseId = lease.leaseId; - let started: RevylStart; - try { - started = await startRevylSession(this.client, lease, { - platform, - appUrl: entry.request.appUrl, - deviceModel: entry.request.deviceModel, - osVersion: entry.request.osVersion, - ...revylCallerBudget(context), - }); - } catch (error) { - // The start failed somewhere this module does not model, and the request may already have gone out, - // so the id it carried is the handle the entry has to end. - started = { - status: 'failed', - error, - rented: { sessionId: revylSessionIdForLease(leaseId) }, - }; - } - if (started.status === 'ready') { - entry.session = started.session; - entry.stop = { runId: started.session.runId, sessionId: started.session.sessionId }; - return { status: 'ready', session: started.session }; - } - const rented = started.rented; - // Revyl settled with a refusal: no session came of this start, so the entry owes nothing and is not - // kept for a stop that would only answer 404. - if (rented === undefined) { - entry.stopConfirmed = true; - this.closeLease(leaseId, entry); - return { status: 'failed', error: started.error }; - } - entry.stop = rented; - // The entry ends what a failed start rented rather than waiting for a release to ask: the device is - // billed from the moment Revyl made it, and this is the first moment its id is known. - const failure = await this.stopOwed(entry, rented); - if (failure === undefined) { - entry.stopConfirmed = true; - this.closeLease(leaseId, entry); - return { status: 'failed', error: started.error }; - } - // What is still billed outranks why the start did not finish, so the stop's failure is reported with - // the session id that has to be ended by hand. - return { - status: 'failed', - error: revylCleanupPending([{ ...rented, failure }], {}, started.error), - }; - } - /** * Joins the start another caller already has running for this lease, within this caller's own budget. * Hanging up here never stops that start and never closes the entry: this caller took no device, so it @@ -371,10 +251,10 @@ class RevylRuntimeImplementation implements ProviderDeviceRuntime { */ private async join( leaseId: string, - entry: RevylLeaseEntry, + entry: RevylLeaseOwner, platform: RevylPlatform, context?: LeaseLifecycleContext, - ): Promise { + ): Promise { const live = entry.session; // A repeat allocation of a session this runtime already holds is that same device, and Revyl is asked // for nothing. @@ -388,54 +268,20 @@ class RevylRuntimeImplementation implements ProviderDeviceRuntime { platform, budget, Math.max(0, (budget.deadline ?? joinedAt) - joinedAt), - entry.stop?.sessionId ?? revylSessionIdForLease(leaseId), + revylSessionIdForLease(leaseId), ), ); - if (gaveUp === undefined) return await entry.start; + if (gaveUp === undefined) return await entry.creation; try { - return await Promise.race([entry.start, gaveUp.promise]); + return await Promise.race([entry.creation, gaveUp.promise]); + } catch (error) { + if (entry.releasing) throw releasingLease(leaseId, error); + throw error; } finally { gaveUp.dispose(); } } - /** - * Ends one entry: waits for its start, sends the `/stop` it owes, and closes it only when Revyl confirms - * the session is gone. Every path that ends a device comes through here, so an unconfirmed stop stays in - * the map for the next release, recovery, or shutdown to retry on the same session. - * - * The wait is what makes a release that lands mid-start stop the device that start rents. A release - * arrives that way when the daemon drops a lease whose caller is gone while a caller still waits for the - * shared start. - */ - private async endLease( - leaseId: string, - entry: RevylLeaseEntry, - ): Promise { - await entry.start; - const target = entry.stop; - // Nothing owed means nothing rented: the start was refused, or its stop is already confirmed. - if (target === undefined) return undefined; - const session = entry.session; - // A failed start has already ended its own session before this point, and a caller that releases such - // a lease must not make Revyl see a second stop for a session already gone. - const failure = entry.stopConfirmed ? undefined : await this.stopOwed(entry, target); - if (failure === undefined) { - entry.stopConfirmed = true; - this.closeLease(leaseId, entry); - } - return { - session: target, - ...(session === undefined ? {} : { data: allocationResult(session) }), - ...(failure === undefined ? {} : { failure }), - }; - } - - /** Removes an entry once Revyl has confirmed its stop, and only while it is still the map's entry. */ - private closeLease(leaseId: string, entry: RevylLeaseEntry): void { - if (this.leases.get(leaseId) === entry) this.leases.delete(leaseId); - } - /** * Ends the lease's session and reports what is still owed, or throws so the daemon keeps the release * pending and retries it. A device Revyl still holds is never reported as released. @@ -448,23 +294,12 @@ class RevylRuntimeImplementation implements ProviderDeviceRuntime { // No entry means nothing is owed: this lease never rented, or a confirmed stop already closed it, and // Revyl is not asked again for a release it has already confirmed. if (!entry) return undefined; - const outcome = await this.endLease(lease.leaseId, entry); + const outcome = await entry.end(); if (outcome === undefined) return undefined; - if (outcome.failure !== undefined) throw revylReleasePending(lease.leaseId, outcome); + if (outcome.cleanup === 'uncertain') throw revylReleasePending(lease.leaseId, outcome); return outcome.data ?? { providerSessionId: outcome.session.sessionId }; } - /** The entry's one in-flight `/stop`, started by the first caller and joined by the rest. */ - private stopOwed( - entry: RevylLeaseEntry, - target: RevylRentedSession, - ): Promise { - entry.stopping ??= this.stopRented(target).finally(() => { - entry.stopping = undefined; - }); - return entry.stopping; - } - /** * One `/stop`, with Revyl's own "no such session" read as released: that device is not rented. The * failure's message is what the caller reports; `undefined` is the confirmation the entry needs. @@ -498,8 +333,16 @@ function revylCallerBudget(context?: LeaseLifecycleContext): RevylCallerBudget { } /** The failure for a release Revyl did not confirm, with the session an operator has to end by hand. */ -function revylReleasePending(leaseId: string, outcome: RevylStopOutcome): AppError { - return revylCleanupPending([{ ...outcome.session, failure: outcome.failure ?? '' }], { leaseId }); +function revylReleasePending( + leaseId: string, + outcome: RevylStopOutcome, + cause?: unknown, +): AppError { + return revylCleanupPending( + [{ ...outcome.session, failure: outcome.cleanup === 'uncertain' ? outcome.failure : '' }], + { leaseId, cleanup: 'uncertain' }, + cause, + ); } /** Recovery and lifecycle calls arrive for one provider's lease; anything else is a routing bug. */ @@ -536,14 +379,16 @@ async function loadRevylPlatformRuntime( }); } -/** - * What the lease lifecycle reports for one session. The core reads `providerSessionId` to record the - * provider session and the device to address the rented device from then on. - */ -function allocationResult(session: RevylSession): Record { - return { - revylRunId: session.runId, - providerSessionId: session.sessionId, - device: session.device, - }; +function releasingLease(leaseId: string, cause?: unknown): AppError { + return new AppError( + 'DEVICE_IN_USE', + 'The Revyl lease is being released.', + { + reason: 'provider-lease-releasing', + leaseId, + retriable: true, + hint: 'Retry after the pending release is confirmed.', + }, + cause, + ); } diff --git a/website/docs/docs/revyl.md b/website/docs/docs/revyl.md index 89c4ce0d44..59f75d3b45 100644 --- a/website/docs/docs/revyl.md +++ b/website/docs/docs/revyl.md @@ -58,6 +58,10 @@ agent-device disconnect `close` ends the app on the device. `disconnect`, and any daemon shutdown, ends the Revyl session, so the device is returned. Revyl also releases an idle session on its own after its idle timeout. +Once release starts, the lease refuses further device commands. If Revyl does not confirm the stop, +agent-device reports the session id and keeps the release pending for retry. Check that session in +Revyl and end it there if it remains running. + ## What works on a Revyl session A Revyl session runs the commands that reach a device through Revyl's device actions: