fix(daemon): let admitted work keep the lease it is working on

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

src/daemon/__tests__/lease-in-flight-work.test.ts

@@ -0,0 +1,85 @@
+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();
+});
+
+// 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<string, unknown> }).entriesByLeaseId;
+  return [...claims.keys()];
+}

src/daemon/__tests__/lease-registry.test.ts

@@ -564,3 +564,93 @@ test('canceling a superseded activation cannot remove its successor or another h
   );
   assert.equal(registry.listHumanControlHolds(authority)[0]?.reason, 'successor');
 });
+
+// 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;
+  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);
+});
+
+// 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<string, unknown> };
+    }
+  ).inFlightWork;
+  return [...work.entriesByLeaseId.keys()];
+}

src/daemon/__tests__/request-execution-scope.test.ts

@@ -509,8 +509,54 @@ test('expired leases remove owned sessions before the next command and free capa
   expect(nextLease.tenantId).toBe('tenant-b');
 });
 
+// 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-');
+  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 +577,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 +600,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 () => {

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> = {},
+): DaemonRequest {
+  return {
+    token: 'test-token',
+    session: 'default',
+    command: 'snapshot',
+    positionals: [],
+    internal: { admittedLease: leaseRegistry.allocateLease(HUMAN_CONTROL_LEASE_REQUEST) },
+    ...overrides,
+  };
+}
+
+// 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 });
+  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([]);
+});