From bad552578a46e96ce6c418667decb36714bd5c5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 12 Sep 2026 19:08:38 +0200 Subject: [PATCH 1/3] fix(daemon): let admitted work keep the lease it is working on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A lease renewed only at admission, so a command slower than its own inactivity TTL expired the lease that was paying for the device it was using. Expiry then tore the provider session down underneath a client still waiting for that same command's result, and every later command on the session reported a lease that was no longer active. The session's own work was the thing that killed it. Admitted work now preserves its lease the way a human-control hold does: while the request is still wanted it defers expiry, and finishing while still wanted renews the lease for its existing TTL from the moment the work ended. Work whose client hung up preserves nothing — it neither defers expiry past that cancellation nor renews the lease when it finally lands — so a handler that ignores its cancellation cannot hold a rented device open. Found while investigating #2509. Not its reported mechanism: a cloud WebDriver connection profile asks for a ten-minute lease, so a one-minute hang cannot starve it. This reaches leases on the daemon's one-minute default. --- docs/adr/0007-remote-device-leases.md | 16 ++++ .../__tests__/lease-in-flight-work.test.ts | 55 ++++++++++++ src/daemon/__tests__/lease-registry.test.ts | 67 +++++++++++++++ .../__tests__/request-execution-scope.test.ts | 63 ++++++++++++-- .../__tests__/request-lease-work.test.ts | 84 +++++++++++++++++++ src/daemon/lease-in-flight-work.ts | 64 ++++++++++++++ src/daemon/lease-registry.ts | 53 +++++++++++- src/daemon/request-execution-scope.ts | 6 +- src/daemon/request-lease-work.ts | 36 ++++++++ 9 files changed, 432 insertions(+), 12 deletions(-) create mode 100644 src/daemon/__tests__/lease-in-flight-work.test.ts create mode 100644 src/daemon/__tests__/request-lease-work.test.ts create mode 100644 src/daemon/lease-in-flight-work.ts create mode 100644 src/daemon/request-lease-work.ts diff --git a/docs/adr/0007-remote-device-leases.md b/docs/adr/0007-remote-device-leases.md index d89566c117..af0e02d6ac 100644 --- a/docs/adr/0007-remote-device-leases.md +++ b/docs/adr/0007-remote-device-leases.md @@ -51,6 +51,22 @@ owning lease expiry. Backend-only leases remain valid for older remote clients, while provider-aware clients get device-level contention and clearer recovery. +## Admitted request work + +A lease renews when a request is admitted and never again while that request runs, so an admitted +command slower than the inactivity TTL used to expire the lease that was paying for its own device +and tear the session down underneath the client still waiting for its result. Admitted work +therefore preserves its lease the way a human-control hold does: while the request is still wanted +it defers expiry, and finishing while still wanted renews the lease for its existing inactivity TTL +from the moment the work ended. A request whose client hung up preserves nothing — it neither defers +expiry past that cancellation nor renews the lease when it finally lands — so a handler that ignores +its cancellation cannot hold a rented device open. + +Which leases this reaches depends on the inactivity TTL the client asked for: the daemon default is +one minute, while a cloud WebDriver connection profile asks for ten. A single command that runs +longer than its own lease is therefore ordinary on the default and only reachable through a profile +on the longer one. + ## Human control Human-control holds coexist with an open remote session. They belong to `LeaseRegistry` and use diff --git a/src/daemon/__tests__/lease-in-flight-work.test.ts b/src/daemon/__tests__/lease-in-flight-work.test.ts new file mode 100644 index 0000000000..4f33bb1ef9 --- /dev/null +++ b/src/daemon/__tests__/lease-in-flight-work.test.ts @@ -0,0 +1,55 @@ +import { test, expect } from 'vitest'; +import { LeaseInFlightWorkRegistry } from '../lease-in-flight-work.ts'; + +test('a pass defers its lease until it is released', () => { + const work = new LeaseInFlightWorkRegistry(); + const pass = work.retain('lease-a', () => true); + + expect(work.isDeferred('lease-a')).toBe(true); + expect(pass.release()).toBe(true); + expect(work.isDeferred('lease-a')).toBe(false); +}); + +// The deferral is a claim that somebody is still waiting. The client hanging up +// ends the claim immediately, without waiting for the abandoned work to unwind. +test('a pass whose request was cancelled stops deferring unreleased', () => { + const work = new LeaseInFlightWorkRegistry(); + let wanted = true; + const pass = work.retain('lease-a', () => wanted); + wanted = false; + + expect(work.isDeferred('lease-a')).toBe(false); + expect(pass.release()).toBe(false); +}); + +// Two requests can work one leased device. Only work still wanted defers, and one +// release must not disturb a pass that outlives it. +test('one wanted pass keeps deferral while an abandoned sibling releases', () => { + const work = new LeaseInFlightWorkRegistry(); + let abandonedWanted = true; + const abandoned = work.retain('lease-a', () => abandonedWanted); + const wanted = work.retain('lease-a', () => true); + abandonedWanted = false; + + expect(abandoned.release()).toBe(false); + expect(work.isDeferred('lease-a')).toBe(true); + expect(wanted.release()).toBe(true); + expect(work.isDeferred('lease-a')).toBe(false); +}); + +test('releasing a pass twice renews nothing twice', () => { + const work = new LeaseInFlightWorkRegistry(); + const pass = work.retain('lease-a', () => true); + + expect(pass.release()).toBe(true); + expect(pass.release()).toBe(false); +}); + +test('passes on different leases defer independently', () => { + const work = new LeaseInFlightWorkRegistry(); + const other = work.retain('lease-b', () => true); + + expect(work.isDeferred('lease-a')).toBe(false); + expect(work.isDeferred('lease-b')).toBe(true); + other.release(); +}); diff --git a/src/daemon/__tests__/lease-registry.test.ts b/src/daemon/__tests__/lease-registry.test.ts index 61dbffbe65..5aa587c0bd 100644 --- a/src/daemon/__tests__/lease-registry.test.ts +++ b/src/daemon/__tests__/lease-registry.test.ts @@ -564,3 +564,70 @@ test('canceling a superseded activation cannot remove its successor or another h ); assert.equal(registry.listHumanControlHolds(authority)[0]?.reason, 'successor'); }); + +// #2509: nothing heartbeats a lease while its request works, so an admitted +// capture that legitimately outruns the lease TTL expired its own lease and lost +// the device it was still holding. In-flight work defers expiry and renews on +// completion, exactly as a human-control hold does. +test('in-flight request work defers expiry and renews the lease when it finishes', async () => { + let now = 0; + const registry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 5_000 }); + const lease = registry.allocateLease(HUMAN_CONTROL_LEASE_REQUEST); + const pass = registry.retainLeaseWork(lease, () => true); + + now = 9_000; + assert.deepEqual(registry.consumeExpiredLeases(), []); + assert.equal(registry.consumeExpiredLease(lease.leaseId), undefined); + assert.equal(registry.listActiveLeases()[0]?.leaseId, lease.leaseId); + + pass.release(); + assert.equal(registry.listActiveLeases()[0]?.expiresAt, 14_000); + now = 14_000; + assert.equal(registry.consumeExpiredLeases()[0]?.leaseId, lease.leaseId); +}); + +// The deferral belongs to work somebody is still waiting for. A request whose +// client hung up stops deferring the moment it is cancelled, and re-earns nothing +// when it finally lands, so a handler that ignores cancellation cannot pin a +// rented device open forever. +test('a cancelled request stops deferring expiry and cannot revive its lease', async () => { + let now = 0; + const registry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 5_000 }); + const lease = registry.allocateLease(HUMAN_CONTROL_LEASE_REQUEST); + let wanted = true; + const pass = registry.retainLeaseWork(lease, () => wanted); + + wanted = false; + now = 9_000; + assert.deepEqual( + registry.consumeExpiredLeases().map((entry) => entry.leaseId), + [lease.leaseId], + ); + pass.release(); + assert.deepEqual(registry.listActiveLeases(), []); +}); + +// The closest negative: two requests on one device, one abandoned. Releasing the +// abandoned work must renew nothing while the wanted work still defers expiry. +test('abandoned work renews nothing while work still wanted holds the lease', async () => { + let now = 0; + const registry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 5_000 }); + const lease = registry.allocateLease(HUMAN_CONTROL_LEASE_REQUEST); + let abandonedWanted = true; + const abandoned = registry.retainLeaseWork(lease, () => abandonedWanted); + const wanted = registry.retainLeaseWork(lease, () => true); + + abandonedWanted = false; + now = 9_000; + assert.equal(registry.listActiveLeases()[0]?.leaseId, lease.leaseId); + + abandoned.release(); + assert.equal( + registry.listActiveLeases()[0]?.expiresAt, + 5_000, + 'abandoned work must not renew the lease it was sitting on', + ); + + wanted.release(); + assert.equal(registry.listActiveLeases()[0]?.expiresAt, 14_000); +}); diff --git a/src/daemon/__tests__/request-execution-scope.test.ts b/src/daemon/__tests__/request-execution-scope.test.ts index 76009c31b9..79b4b70f8b 100644 --- a/src/daemon/__tests__/request-execution-scope.test.ts +++ b/src/daemon/__tests__/request-execution-scope.test.ts @@ -509,8 +509,53 @@ test('expired leases remove owned sessions before the next command and free capa expect(nextLease.tenantId).toBe('tenant-b'); }); +// #2509: the slowest command in a cloud session killed every command after it. +// Nothing heartbeats a lease while its request works, so one capture that ran +// past the lease TTL expired its own lease, which tore the provider session down +// under the client that was still waiting for it. +test('an admitted request that outlives the lease TTL keeps its lease and session', async () => { + let now = 1_000; + const sessionStore = makeSessionStore('agent-device-request-scope-'); + const leaseRegistry = new LeaseRegistry({ + defaultLeaseTtlMs: 10, + minLeaseTtlMs: 1, + now: () => now, + }); + const lease = leaseRegistry.allocateLease({ tenantId: 'tenant-a', runId: 'run-1' }); + sessionStore.set( + 'default', + makeIosSession('default', { + lease: { + leaseId: lease.leaseId, + tenantId: lease.tenantId, + runId: lease.runId, + leaseBackend: lease.backend, + expiresAt: lease.expiresAt, + }, + }), + ); + + const slow = await createRequestExecutionScope({ + req: makeRequest({ command: 'snapshot' }), + sessionStore, + leaseRegistry, + }); + // The capture is still being waited on when it crosses the TTL, as a cloud + // page-source read does on a screen that never goes idle. + expect(await slow.runLocked(async () => (now = 1_011))).toBe(1_011); + + const next = await createRequestExecutionScope({ + req: makeRequest({ command: 'screenshot' }), + sessionStore, + leaseRegistry, + }); + expect(await next.runLocked(async () => 'ran')).toBe('ran'); + expect(sessionStore.get('default')).toBeDefined(); +}); + test('expired leased session cleanup waits for the request execution lock', async () => { let now = 1_000; + const requestId = 'request-scope-holds-execution-lock'; const sessionStore = makeSessionStore('agent-device-request-scope-'); const leaseRegistry = new LeaseRegistry({ defaultLeaseTtlMs: 10, @@ -531,7 +576,7 @@ test('expired leased session cleanup waits for the request execution lock', asyn }), ); const first = await createRequestExecutionScope({ - req: makeRequest({ command: 'click' }), + req: makeRequest({ command: 'click', meta: { requestId } }), sessionStore, leaseRegistry, }); @@ -554,16 +599,24 @@ test('expired leased session cleanup waits for the request execution lock', asyn }), ); await firstEnteredPromise; + // The client walked away from this request. Abandoned work no longer defers the + // expiry it is sitting on, which is what makes this case about the lock and not + // about in-flight lease liveness. + markRequestCanceled(requestId); now = 1_011; const secondRun = second.runLocked(async () => 'second'); await new Promise((resolve) => setTimeout(resolve, 20)); expect(sessionStore.get('default')).toBeDefined(); - releaseFirst(); - await firstRun; - await expect(secondRun).resolves.toBe('second'); - expect(sessionStore.get('default')).toBeUndefined(); + try { + releaseFirst(); + await firstRun; + await expect(secondRun).resolves.toBe('second'); + expect(sessionStore.get('default')).toBeUndefined(); + } finally { + clearRequestCanceled(requestId); + } }); test('tenant lease rejection flushes diagnostics into the effective session request log', async () => { diff --git a/src/daemon/__tests__/request-lease-work.test.ts b/src/daemon/__tests__/request-lease-work.test.ts new file mode 100644 index 0000000000..25e80a2873 --- /dev/null +++ b/src/daemon/__tests__/request-lease-work.test.ts @@ -0,0 +1,84 @@ +import { test, expect } from 'vitest'; +import { + clearRequestAbortRegistration, + markRequestCanceled, + registerRequestAbort, +} from '@agent-device/host-kit/request'; +import type { DaemonRequest } from '../daemon-request.ts'; +import { LeaseRegistry } from '../lease-registry.ts'; +import { runAdmittedLeaseWork } from '../request-lease-work.ts'; +import { HUMAN_CONTROL_LEASE_REQUEST } from './human-control-fixtures.ts'; + +function admittedRequest( + leaseRegistry: LeaseRegistry, + overrides: Partial = {}, +): DaemonRequest { + return { + token: 'test-token', + session: 'default', + command: 'snapshot', + positionals: [], + internal: { admittedLease: leaseRegistry.allocateLease(HUMAN_CONTROL_LEASE_REQUEST) }, + ...overrides, + }; +} + +// #2509: a capture that ran past the lease TTL expired the lease that was paying +// for the device, so the session died underneath the client still waiting for it. +test('admitted work that outlives the lease TTL keeps the lease it is working on', async () => { + let now = 0; + const leaseRegistry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 5_000 }); + const req = admittedRequest(leaseRegistry); + + const worked = await runAdmittedLeaseWork({ + leaseRegistry, + req, + task: async () => { + now = 9_000; + expect(leaseRegistry.listActiveLeases()).toHaveLength(1); + return 'captured'; + }, + }); + + expect(worked).toBe('captured'); + expect(leaseRegistry.listActiveLeases()[0]?.expiresAt).toBe(14_000); +}); + +// The pass protects work somebody is still waiting for, and nothing else. Once the +// client hangs up, work that lands later re-earns no lease: a handler that ignores +// its cancellation cannot pin a rented device open. +test('work nobody waited for renews nothing once its lease fell due', async () => { + let now = 0; + const leaseRegistry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 5_000 }); + const requestId = 'request-2509-abandoned'; + const registration = registerRequestAbort(requestId); + try { + const req = admittedRequest(leaseRegistry, { meta: { requestId } }); + const worked = await runAdmittedLeaseWork({ + leaseRegistry, + req, + task: async () => { + now = 9_000; + markRequestCanceled(requestId); + return 'late'; + }, + }); + + expect(worked).toBe('late'); + expect(leaseRegistry.listActiveLeases()).toEqual([]); + } finally { + clearRequestAbortRegistration(registration); + } +}); + +test('a request admitted without a lease runs its work untouched', async () => { + const leaseRegistry = new LeaseRegistry(); + const result = await runAdmittedLeaseWork({ + leaseRegistry, + req: { token: 'test-token', session: 'default', command: 'status', positionals: [] }, + task: async () => 'ran', + }); + + expect(result).toBe('ran'); + expect(leaseRegistry.listActiveLeases()).toEqual([]); +}); diff --git a/src/daemon/lease-in-flight-work.ts b/src/daemon/lease-in-flight-work.ts new file mode 100644 index 0000000000..095e032175 --- /dev/null +++ b/src/daemon/lease-in-flight-work.ts @@ -0,0 +1,64 @@ +/** + * Which leased devices have admitted request work running on them right now. + * + * A remote lease renews when a request is admitted and never again while that + * request works, so work that legitimately outlives the lease TTL — a cloud + * page-source read on a screen that never goes idle (#2509) — expired the very + * lease paying for the device, and the session with it. + */ + +/** Whether the client behind one request is still waiting for its result. */ +export type LeaseWorkWanted = () => boolean; + +/** A claim on one leased device for the duration of one admitted request's work. */ +export type LeaseWorkPass = Readonly<{ + leaseId: string; + /** + * Ends the pass. Reports whether the work was still wanted when it ended, which + * is what entitles it to renew the lease it just worked on. + */ + release(): boolean; +}>; + +type LeaseWorkEntry = { + readonly wanted: LeaseWorkWanted; + released: boolean; +}; + +/** + * A pass defers its lease's expiry for as long as the request that opened it is + * still wanted. The moment that client hangs up the pass defers nothing, so a + * handler that ignores its cancellation cannot hold a rented device open. + */ +export class LeaseInFlightWorkRegistry { + private readonly entriesByLeaseId = new Map>(); + + retain(leaseId: string, wanted: LeaseWorkWanted): LeaseWorkPass { + const entry: LeaseWorkEntry = { wanted, released: false }; + const entries = this.entriesByLeaseId.get(leaseId) ?? new Set(); + entries.add(entry); + this.entriesByLeaseId.set(leaseId, entries); + return { leaseId, release: () => this.releasePass(entries, entry) }; + } + + /** True while any pass on this lease is still wanted. Unwanted passes defer nothing. */ + isDeferred(leaseId: string): boolean { + const entries = this.entriesByLeaseId.get(leaseId); + if (!entries) return false; + for (const entry of entries) { + if (entry.wanted()) continue; + entries.delete(entry); + } + if (entries.size > 0) return true; + this.entriesByLeaseId.delete(leaseId); + return false; + } + + /** Idempotent: only the first release of a pass can report its work as wanted. */ + private releasePass(entries: Set, entry: LeaseWorkEntry): boolean { + if (entry.released) return false; + entry.released = true; + entries.delete(entry); + return entry.wanted(); + } +} diff --git a/src/daemon/lease-registry.ts b/src/daemon/lease-registry.ts index 35e9e89696..95f29faea8 100644 --- a/src/daemon/lease-registry.ts +++ b/src/daemon/lease-registry.ts @@ -26,6 +26,11 @@ import { leaseRunBindingKey, } from './lease-registry-scope.ts'; import { DeviceMutationDrain } from './device-mutation-drain.ts'; +import { + type LeaseWorkPass, + type LeaseWorkWanted, + LeaseInFlightWorkRegistry, +} from './lease-in-flight-work.ts'; import { type HumanControlAuthority, type HumanControlHoldInput, @@ -42,6 +47,7 @@ type OwnedHumanControlHold = { hold: HumanControlHold; ownerLeaseId?: string }; export class LeaseRegistry { private readonly holdsByDevice = new Map>(); private readonly mutations = new DeviceMutationDrain(); + private readonly inFlightWork = new LeaseInFlightWorkRegistry(); private readonly leases = new Map(); private readonly runBindings = new Map(); private readonly deviceBindings = new Map(); @@ -111,6 +117,27 @@ export class LeaseRegistry { return this.refreshLease(lease, leaseTtlMs); } + /** + * Protects a lease while an admitted request works on its device. A lease renews + * when a request is admitted and never again while that request runs, so without + * this the slowest command in a session expired the lease that was paying for + * the device and tore the session down under the client still waiting for it + * (#2509). Releasing a still-wanted pass renews the lease the way releasing a + * human-control hold does; a pass whose request was cancelled protects nothing. + * `wanted` is how the caller reports that its client is still waiting. + */ + retainLeaseWork(lease: Pick, wanted: LeaseWorkWanted): LeaseWorkPass { + const pass = this.inFlightWork.retain(lease.leaseId, wanted); + return { + leaseId: pass.leaseId, + release: () => { + const renewed = pass.release(); + if (renewed) this.refreshProtectedLease(lease.leaseId, this.now()); + return renewed; + }, + }; + } + releaseLease(request: ReleaseLeaseRequest): { released: boolean; lease?: DeviceLease } { const lease = this.getLease(request); if (!lease) { @@ -359,6 +386,18 @@ export class LeaseRegistry { return key !== undefined && this.holdsByDevice.has(key); } + /** + * What keeps a past-due lease alive: a human holding the device, or an admitted + * request still working on it while its client still wants the result. + */ + private isLeaseProtected(lease: DeviceLease, now: number): boolean { + return ( + lease.expiresAt > now || + this.hasHumanControl(lease) || + this.inFlightWork.isDeferred(lease.leaseId) + ); + } + private expireHumanControlHolds(): void { const now = this.now(); for (const [key, holds] of this.holdsByDevice) { @@ -376,7 +415,15 @@ export class LeaseRegistry { } private refreshHeldLease(key: string, at: number): void { - const leaseId = this.deviceBindings.get(key); + this.refreshProtectedLease(this.deviceBindings.get(key), at); + } + + /** + * The one rule for a device resource that stops holding a lease: the lease is + * renewed for its own TTL from the instant the resource let go, never from a + * later sweep, so work nobody waited for cannot revive an abandoned lease. + */ + private refreshProtectedLease(leaseId: string | undefined, at: number): void { const lease = leaseId ? this.leases.get(leaseId) : undefined; if (lease) { this.refreshLease( @@ -392,7 +439,7 @@ export class LeaseRegistry { const now = this.now(); const expired: DeviceLease[] = []; for (const lease of this.leases.values()) { - if (lease.expiresAt > now || this.hasHumanControl(lease)) continue; + if (this.isLeaseProtected(lease, now)) continue; this.leases.delete(lease.leaseId); this.unbindLease(lease, lease.expiresAt); const expiredLease = { ...lease }; @@ -407,7 +454,7 @@ export class LeaseRegistry { const normalizedLeaseId = normalizeLeaseId(leaseId); if (!normalizedLeaseId) return undefined; const lease = this.leases.get(normalizedLeaseId); - if (!lease || lease.expiresAt > this.now() || this.hasHumanControl(lease)) { + if (!lease || this.isLeaseProtected(lease, this.now())) { return undefined; } this.leases.delete(lease.leaseId); diff --git a/src/daemon/request-execution-scope.ts b/src/daemon/request-execution-scope.ts index 06a70ec011..20189b63a7 100644 --- a/src/daemon/request-execution-scope.ts +++ b/src/daemon/request-execution-scope.ts @@ -25,8 +25,8 @@ import { createRequestExecutionLocks } from './request-execution-locks.ts'; import { throwIfRequestCanceled } from '@agent-device/host-kit/request'; import { finalizeDaemonResponse } from './request-finalization.ts'; import { refreshRecordingHealth } from './request-recording-health.ts'; +import { runAdmittedLeaseWork } from './request-lease-work.ts'; import { - isHumanControlMutation, shouldBlockForInvalidRecording, shouldLockSessionExecution, shouldValidateSessionSelector, @@ -239,9 +239,7 @@ export async function createRequestExecutionScope(params: { providerAppCatalog: params.providerAppCatalog, }); scope.req = scopedReq; - return isHumanControlMutation(scopedReq) - ? await leaseRegistry.runDeviceMutation(scopedReq.internal?.admittedLease, task) - : await task(); + return await runAdmittedLeaseWork({ leaseRegistry, req: scopedReq, task }); }, runLocked: async (task) => { throwIfRequestCanceled(scopedReq.meta?.requestId); diff --git a/src/daemon/request-lease-work.ts b/src/daemon/request-lease-work.ts new file mode 100644 index 0000000000..4f5783fb37 --- /dev/null +++ b/src/daemon/request-lease-work.ts @@ -0,0 +1,36 @@ +import { isRequestCanceled } from '@agent-device/host-kit/request'; +import { isHumanControlMutation } from './daemon-command-registry.ts'; +import type { LeaseRegistry } from './lease-registry.ts'; +import type { DaemonRequest } from './daemon-request.ts'; + +/** + * Runs one admitted request's work under the protection of its lease. + * + * A lease renews when a request is admitted and never again while that request + * works, so the slowest command in a session used to expire the very lease that + * was paying for the device and tear the session down underneath the client still + * waiting for its result (#2509). The pass defers expiry only while the work is + * still wanted, which the request-cancel registry already knows: once the client + * hangs up the request protects nothing, so a handler that ignores its + * cancellation cannot hold a rented device open. + */ +export async function runAdmittedLeaseWork( + params: Readonly<{ + leaseRegistry: LeaseRegistry; + req: DaemonRequest; + task: () => Promise; + }>, +): Promise { + const { leaseRegistry, req, task } = params; + const lease = req.internal?.admittedLease; + if (!lease) return await task(); + const requestId = req.meta?.requestId; + const work = leaseRegistry.retainLeaseWork(lease, () => !isRequestCanceled(requestId)); + try { + return await (isHumanControlMutation(req) + ? leaseRegistry.runDeviceMutation(lease, task) + : task()); + } finally { + work.release(); + } +} From 41c6de5d520049464e6509a5c606afad467c6b58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 12 Sep 2026 20:41:55 +0200 Subject: [PATCH 2/3] fix(daemon): drop a released lease's work claims instead of leaving them empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A completed pass emptied its set but left the key behind, and only the expiry sweep removed keys — which never reads a released lease again. Every connect-and-close that ran a command on a leased device left another permanent entry in the daemon. Releasing the last pass now removes its lease's entry, and releasing the lease drops its claims outright and disarms the passes still running on them, so work that outlives its own lease renews nothing. --- .../__tests__/lease-in-flight-work.test.ts | 30 +++++++++++++++++++ src/daemon/__tests__/lease-registry.test.ts | 23 ++++++++++++++ src/daemon/lease-in-flight-work.ts | 30 +++++++++++++++---- src/daemon/lease-registry.ts | 1 + 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/src/daemon/__tests__/lease-in-flight-work.test.ts b/src/daemon/__tests__/lease-in-flight-work.test.ts index 4f33bb1ef9..c1b84bb1eb 100644 --- a/src/daemon/__tests__/lease-in-flight-work.test.ts +++ b/src/daemon/__tests__/lease-in-flight-work.test.ts @@ -53,3 +53,33 @@ test('passes on different leases defer independently', () => { expect(work.isDeferred('lease-b')).toBe(true); other.release(); }); + +// Leases churn with every connect and close, and a released lease is never read by +// the expiry sweep again. A key left behind per released lease is a permanent claim +// on memory the daemon can never reclaim. +test('releasing the last pass leaves no claim recorded for its lease', () => { + const work = new LeaseInFlightWorkRegistry(); + const first = work.retain('lease-a', () => true); + const second = work.retain('lease-b', () => true); + + first.release(); + second.release(); + + expect(claimedLeaseIds(work)).toEqual([]); +}); + +// Work that outlives its own lease renews nothing, and holds no key either. +test('forgetting a lease releases its claims and disarms the passes running on it', () => { + const work = new LeaseInFlightWorkRegistry(); + const pass = work.retain('lease-a', () => true); + + work.forget('lease-a'); + + expect(claimedLeaseIds(work)).toEqual([]); + expect(pass.release()).toBe(false); +}); + +function claimedLeaseIds(work: LeaseInFlightWorkRegistry): string[] { + const claims = (work as unknown as { entriesByLeaseId: Map }).entriesByLeaseId; + return [...claims.keys()]; +} diff --git a/src/daemon/__tests__/lease-registry.test.ts b/src/daemon/__tests__/lease-registry.test.ts index 5aa587c0bd..97d7c28df6 100644 --- a/src/daemon/__tests__/lease-registry.test.ts +++ b/src/daemon/__tests__/lease-registry.test.ts @@ -631,3 +631,26 @@ test('abandoned work renews nothing while work still wanted holds the lease', as wanted.release(); assert.equal(registry.listActiveLeases()[0]?.expiresAt, 14_000); }); + +// The ordinary release path, not expiry: a released lease is never read by the +// expiry sweep again, so a work claim left against it would never be cleaned. +test('releasing a lease drops the work claims recorded against it', () => { + const registry = new LeaseRegistry({ defaultLeaseTtlMs: 5_000 }); + const lease = registry.allocateLease({ tenantId: 'tenant-a', runId: 'run-9' }); + const pass = registry.retainLeaseWork(lease, () => true); + + registry.releaseLease({ leaseId: lease.leaseId }); + pass.release(); + + assert.deepEqual(inFlightClaimKeys(registry), []); + assert.equal(registry.listActiveLeases().length, 0, 'a released lease must not come back'); +}); + +function inFlightClaimKeys(registry: LeaseRegistry): string[] { + const work = ( + registry as unknown as { + inFlightWork: { entriesByLeaseId: Map }; + } + ).inFlightWork; + return [...work.entriesByLeaseId.keys()]; +} diff --git a/src/daemon/lease-in-flight-work.ts b/src/daemon/lease-in-flight-work.ts index 095e032175..887be0988a 100644 --- a/src/daemon/lease-in-flight-work.ts +++ b/src/daemon/lease-in-flight-work.ts @@ -2,9 +2,10 @@ * Which leased devices have admitted request work running on them right now. * * A remote lease renews when a request is admitted and never again while that - * request works, so work that legitimately outlives the lease TTL — a cloud - * page-source read on a screen that never goes idle (#2509) — expired the very - * lease paying for the device, and the session with it. + * request works, so a command that legitimately outlives its lease's inactivity + * TTL expired the very lease paying for the device it was using, and the session + * with it. The daemon's default inactivity TTL is one minute; a cloud WebDriver + * connection profile asks for ten, which decides which leases this reaches. */ /** Whether the client behind one request is still waiting for its result. */ @@ -38,7 +39,21 @@ export class LeaseInFlightWorkRegistry { const entries = this.entriesByLeaseId.get(leaseId) ?? new Set(); entries.add(entry); this.entriesByLeaseId.set(leaseId, entries); - return { leaseId, release: () => this.releasePass(entries, entry) }; + return { leaseId, release: () => this.releasePass(leaseId, entries, entry) }; + } + + /** + * Drops every claim on a lease that no longer exists, and marks them released so + * work that outlives its own lease renews nothing — including a lease later + * allocated under the same id. + */ + forget(leaseId: string): void { + const entries = this.entriesByLeaseId.get(leaseId); + if (!entries) return; + for (const entry of entries) { + entry.released = true; + } + this.entriesByLeaseId.delete(leaseId); } /** True while any pass on this lease is still wanted. Unwanted passes defer nothing. */ @@ -55,10 +70,15 @@ export class LeaseInFlightWorkRegistry { } /** Idempotent: only the first release of a pass can report its work as wanted. */ - private releasePass(entries: Set, entry: LeaseWorkEntry): boolean { + private releasePass( + leaseId: string, + entries: Set, + entry: LeaseWorkEntry, + ): boolean { if (entry.released) return false; entry.released = true; entries.delete(entry); + if (entries.size === 0) this.entriesByLeaseId.delete(leaseId); return entry.wanted(); } } diff --git a/src/daemon/lease-registry.ts b/src/daemon/lease-registry.ts index 95f29faea8..b8c722b854 100644 --- a/src/daemon/lease-registry.ts +++ b/src/daemon/lease-registry.ts @@ -145,6 +145,7 @@ export class LeaseRegistry { } this.leases.delete(lease.leaseId); this.unbindLease(lease); + this.inFlightWork.forget(lease.leaseId); return { released: true, lease }; } From 25c563d02eb3a1fac188dac2e9fbb81f6b4bde4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Sat, 12 Sep 2026 20:46:10 +0200 Subject: [PATCH 3/3] docs(daemon): state the lease invariant without borrowing #2509's cause Four comments told the report's story as though it were this mechanism. A cloud WebDriver connection profile asks for a ten-minute lease, so the reported one-minute hang cannot have expired anything. The invariant stands on its own; where it came from and which leases it reaches belong in ADR 0007 and the commit, not in each test. --- src/daemon/__tests__/lease-registry.test.ts | 6 +++--- src/daemon/__tests__/request-execution-scope.test.ts | 9 +++++---- src/daemon/__tests__/request-lease-work.test.ts | 4 ++-- src/daemon/request-lease-work.ts | 2 +- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/daemon/__tests__/lease-registry.test.ts b/src/daemon/__tests__/lease-registry.test.ts index 97d7c28df6..fc3d576d5e 100644 --- a/src/daemon/__tests__/lease-registry.test.ts +++ b/src/daemon/__tests__/lease-registry.test.ts @@ -565,9 +565,9 @@ test('canceling a superseded activation cannot remove its successor or another h assert.equal(registry.listHumanControlHolds(authority)[0]?.reason, 'successor'); }); -// #2509: nothing heartbeats a lease while its request works, so an admitted -// capture that legitimately outruns the lease TTL expired its own lease and lost -// the device it was still holding. In-flight work defers expiry and renews on +// Nothing heartbeats a lease while its request works, so an admitted capture +// that legitimately outruns the lease TTL expires its own lease and loses the +// device it was still holding. In-flight work defers expiry and renews on // completion, exactly as a human-control hold does. test('in-flight request work defers expiry and renews the lease when it finishes', async () => { let now = 0; diff --git a/src/daemon/__tests__/request-execution-scope.test.ts b/src/daemon/__tests__/request-execution-scope.test.ts index 79b4b70f8b..e9e23b0ff5 100644 --- a/src/daemon/__tests__/request-execution-scope.test.ts +++ b/src/daemon/__tests__/request-execution-scope.test.ts @@ -509,10 +509,11 @@ test('expired leases remove owned sessions before the next command and free capa expect(nextLease.tenantId).toBe('tenant-b'); }); -// #2509: the slowest command in a cloud session killed every command after it. -// Nothing heartbeats a lease while its request works, so one capture that ran -// past the lease TTL expired its own lease, which tore the provider session down -// under the client that was still waiting for it. +// A lease renewed only at admission lets one command slower than its inactivity TTL +// expire the lease paying for the device it is using, and expiry then tears the +// provider session down under the client still waiting for that same command. Found +// while investigating #2509, whose cloud session ran on a ten-minute lease and so +// lost its session some other way. test('an admitted request that outlives the lease TTL keeps its lease and session', async () => { let now = 1_000; const sessionStore = makeSessionStore('agent-device-request-scope-'); diff --git a/src/daemon/__tests__/request-lease-work.test.ts b/src/daemon/__tests__/request-lease-work.test.ts index 25e80a2873..16dc991578 100644 --- a/src/daemon/__tests__/request-lease-work.test.ts +++ b/src/daemon/__tests__/request-lease-work.test.ts @@ -23,8 +23,8 @@ function admittedRequest( }; } -// #2509: a capture that ran past the lease TTL expired the lease that was paying -// for the device, so the session died underneath the client still waiting for it. +// A capture that runs past the lease TTL used to expire the lease paying for the +// device, so the session died underneath the client still waiting for it. test('admitted work that outlives the lease TTL keeps the lease it is working on', async () => { let now = 0; const leaseRegistry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 5_000 }); diff --git a/src/daemon/request-lease-work.ts b/src/daemon/request-lease-work.ts index 4f5783fb37..e7d6582b9e 100644 --- a/src/daemon/request-lease-work.ts +++ b/src/daemon/request-lease-work.ts @@ -9,7 +9,7 @@ import type { DaemonRequest } from './daemon-request.ts'; * A lease renews when a request is admitted and never again while that request * works, so the slowest command in a session used to expire the very lease that * was paying for the device and tear the session down underneath the client still - * waiting for its result (#2509). The pass defers expiry only while the work is + * waiting for its result. The pass defers expiry only while the work is * still wanted, which the request-cancel registry already knows: once the client * hangs up the request protects nothing, so a handler that ignores its * cancellation cannot hold a rented device open.