fix(daemon): stop provider lease releases after shutdown closes the releaser

docs/adr/0007-remote-device-leases.md

@@ -91,7 +91,10 @@ A daemon that starts and finds this journal hands each lease in it to the expire
 It reads both lease records only after it holds the daemon registration for the state directory, because until then they can belong to a daemon that is still running.
 The lease itself is not restored, and its controller connects again.
 Restoring the live lease was rejected: the sessions and device claims of the dead daemon are gone, so a restored lease would name a device that no session can reach.
-A lease that leaves the registry for expiry stays in the journal until its expired-lease record is durable, and a journal write that fails is retried.
+A lease that leaves the registry for expiry stays in the journal until its expired-lease record is durable, and a journal write that fails is retried until this daemon shuts down.
+If a journal retry is pending, daemon shutdown makes one final journal write. It does not schedule another journal retry.
+A daemon that has shut down writes lease records only synchronously. It arms no timer for them, because a timer could fire after that daemon released its registration, when the records belong to the next daemon.
+After the daemon shuts its lease releaser down, that releaser refuses every further provider-backed release. It still writes the expired-lease record of a recoverable lease, so the next daemon inherits it. If no record names the lease, the daemon logs the refusal with its lease provider, because no daemon will end that device.
 Two routes still let the registry forget a provider lease while its device may be rented. The journal and expiry recovery both follow the registry, so neither covers it, and only the provider runtime still holding the device can end it:
 
 - A failed first allocation releases the registry entry without a provider release, while its provider start may still run.

packages/daemon/src/server/__tests__/provider-lease-expiry.test.ts

@@ -150,10 +150,11 @@ test('shutdown drain reports releases completed before and during its final retr
   });
 
   try {
-    await releaser.release(releasedBeforeShutdown);
+    await expect(releaser.release(releasedBeforeShutdown)).resolves.toEqual({ status: 'released' });
     releaser.beginShutdown();
-    await releaser.release(releasedDuringShutdown);
-    await releaser.release(lease);
+    // This one's first attempt fails, so the releaser keeps it and the drain retries it.
+    await expect(releaser.release(releasedDuringShutdown)).resolves.toEqual({ status: 'retrying' });
+    await expect(releaser.release(lease)).resolves.toEqual({ status: 'released' });
     const drained = await releaser.drain(10);
 
     expect(release).toHaveBeenCalledTimes(4);
@@ -389,6 +390,144 @@ test('daemon startup recovers inherited lease records only after it holds the re
   expect(source.indexOf('expiredProviderLeaseReleaser.retryPending()')).toBe(-1);
 });
 
+test('a release that fails after shutdown is not retried and stays durable', async () => {
+  vi.useFakeTimers();
+  const stateDir = mkdtempForTestSync('agent-device-expired-provider-lease-');
+  const lease = new LeaseRegistry().allocateLease({
+    tenantId: 'tenant-a',
+    runId: 'run-1',
+    leaseProvider: 'limrun',
+  });
+  let rejectHeldRelease: ((error: Error) => void) | undefined;
+  const recoverExpiredLease = vi.fn(
+    () =>
+      new Promise<void>((_resolve, reject) => {
+        rejectHeldRelease = reject;
+      }),
+  );
+  const releaser = createExpiredProviderLeaseReleaser({
+    recoverExpiredLease,
+    stateDir,
+    recoverableProviderIds: ['limrun'],
+    retryDelayMs: 1_000,
+  });
+
+  try {
+    releaser.beginShutdown();
+    // The provider call is still in flight when the drain budget runs out, so its outcome is unknown.
+    const admitted = releaser.release(lease);
+    const drained = releaser.drain(10);
+    await vi.advanceTimersByTimeAsync(10);
+    expect(await drained).toEqual({ pending: [lease], unknown: [lease], released: [] });
+    expect(recoverExpiredLease).toHaveBeenCalledTimes(1);
+
+    // Plugins are disposed here. The provider call this daemon is waiting on then fails.
+    releaser.shutdown();
+    rejectHeldRelease!(new Error('provider plugin disposed'));
+    await vi.advanceTimersByTimeAsync(60_000);
+
+    expect(recoverExpiredLease).toHaveBeenCalledTimes(1);
+    // Nothing is armed to call the provider again, and nothing is armed to write a lease record.
+    expect(vi.getTimerCount()).toBe(0);
+    const record = JSON.parse(
+      fs.readFileSync(`${stateDir}/expired-provider-leases.json`, 'utf8'),
+    ) as { leases: DeviceLease[] };
+    expect(record.leases.map((entry) => entry.leaseId)).toEqual([lease.leaseId]);
+    // The provider call this daemon was waiting on failed once the releaser had closed, so the lease
+    // stayed pending, and nothing here will attempt it again. The record is what carries it forward.
+    await expect(admitted).resolves.toEqual({ status: 'refused', reason: 'closed', durable: true });
+  } finally {
+    releaser.shutdown();
+    vi.useRealTimers();
+    fs.rmSync(stateDir, { recursive: true, force: true });
+  }
+});
+
+test('release after shutdown starts no provider call and retries nothing', async () => {
+  vi.useFakeTimers();
+  const stateDir = mkdtempForTestSync('agent-device-expired-provider-lease-');
+  const liveOnlyLease = new LeaseRegistry().allocateLease({
+    tenantId: 'tenant-a',
+    runId: 'run-1',
+    leaseProvider: 'browserstack',
+  });
+  const recoverableLease = new LeaseRegistry().allocateLease({
+    tenantId: 'tenant-a',
+    runId: 'run-2',
+    leaseProvider: 'limrun',
+  });
+  const release = vi.fn(async (_lease: DeviceLease) => ({}));
+  const recoverExpiredLease = vi.fn(async (_lease: DeviceLease) => undefined);
+  const releaser = createExpiredProviderLeaseReleaser({
+    leaseLifecycleProvider: { release },
+    providerRuntimeIds: ['browserstack'],
+    recoverExpiredLease,
+    stateDir,
+    recoverableProviderIds: ['limrun'],
+    retryDelayMs: 10,
+  });
+
+  try {
+    releaser.shutdown();
+    await expect(releaser.release(liveOnlyLease)).resolves.toEqual({
+      status: 'refused',
+      reason: 'closed',
+      durable: false,
+    });
+    await expect(releaser.release(recoverableLease)).resolves.toEqual({
+      status: 'refused',
+      reason: 'closed',
+      durable: true,
+    });
+    await vi.advanceTimersByTimeAsync(60_000);
+
+    expect(release).not.toHaveBeenCalled();
+    expect(recoverExpiredLease).not.toHaveBeenCalled();
+    // What a closed daemon cannot release, it leaves durable for the next one.
+    const record = JSON.parse(
+      fs.readFileSync(`${stateDir}/expired-provider-leases.json`, 'utf8'),
+    ) as { leases: DeviceLease[] };
+    expect(record.leases.map((entry) => entry.leaseId)).toEqual([recoverableLease.leaseId]);
+  } finally {
+    releaser.shutdown();
+    vi.useRealTimers();
+    fs.rmSync(stateDir, { recursive: true, force: true });
+  }
+});
+
+test('a lease that expires after shutdown is still recovered by the next daemon', async () => {
+  const stateDir = mkdtempForTestSync('agent-device-live-provider-lease-');
+  let now = 1_000;
+  const recoverExpiredLease = vi.fn(async (_lease: DeviceLease) => undefined);
+  const options = { recoverExpiredLease, stateDir, recoverableProviderIds: ['revyl'] };
+  const daemon = createExpiredProviderLeaseReleaser(options);
+  const registry = new LeaseRegistry({
+    defaultLeaseTtlMs: 10,
+    minLeaseTtlMs: 1,
+    now: () => now,
+    onLeaseExpired: (lease) => void daemon.release(lease),
+    onProviderLeasesChanged: (leases) => daemon.recordLiveLeases(leases),
+  });
+
+  try {
+    const lease = registry.allocateLease({ tenantId: 't', runId: 'run-1', leaseProvider: 'revyl' });
+    daemon.shutdown();
+    // An abandoned request let this lease expire once this daemon had stopped releasing.
+    now = 1_011;
+    registry.consumeExpiredLease(lease.leaseId);
+
+    expect(recoverExpiredLease).not.toHaveBeenCalled();
+
+    const restartedDaemon = createExpiredProviderLeaseReleaser(options);
+    await restartedDaemon.recoverInherited();
+    expect(recoverExpiredLease).toHaveBeenCalledWith(lease);
+    restartedDaemon.shutdown();
+  } finally {
+    daemon.shutdown();
+    fs.rmSync(stateDir, { recursive: true, force: true });
+  }
+});
+
 test('an unreadable live journal is reported with its path and left on disk', async () => {
   const stateDir = mkdtempForTestSync('agent-device-live-provider-lease-');
   const journalPath = `${stateDir}/live-provider-leases.json`;

packages/daemon/src/server/http/daemon-lease-finalizer.test.ts

@@ -3,6 +3,7 @@ import path from 'node:path';
 import { expect, test, vi } from 'vitest';
 import { LeaseRegistry } from '../../resources/lease-registry.ts';
 import type { DeviceLease } from '@agent-device/contracts/device';
+import { withDaemonDiagnosticsScope } from '@agent-device/daemon-contracts/daemon-diagnostics-scope';
 import { createExpiredProviderLeaseReleaser } from '../provider-lease-expiry.ts';
 import { finalizeDaemonLeases } from './daemon-lease-finalizer.ts';
 import { mkdtempForTestSync } from '@agent-device/test-kit';
@@ -52,6 +53,62 @@ test('journals and bounds a hung recoverable lease release before the final drai
   }
 });
 
+test('a release the closed releaser refused is reported and not counted as completed', async () => {
+  const stateDir = mkdtempForTestSync('agent-device-daemon-lease-finalizer-');
+  const leaseRegistry = new LeaseRegistry();
+  const lease = leaseRegistry.allocateLease({
+    tenantId: 'tenant-a',
+    runId: 'run-1',
+    leaseProvider: 'browserstack',
+  });
+  const release = vi.fn(async (_lease: DeviceLease) => ({}));
+  const expiredProviderLeaseReleaser = createExpiredProviderLeaseReleaser({
+    leaseLifecycleProvider: { release },
+    providerRuntimeIds: ['browserstack'],
+  });
+  // The drain budget ran out and the releaser closed before this lease was offered to it.
+  expiredProviderLeaseReleaser.beginShutdown();
+  expiredProviderLeaseReleaser.shutdown();
+
+  const logPath = path.join(stateDir, 'daemon.log');
+  try {
+    const outcomes = await withDaemonDiagnosticsScope({ logPath }, () =>
+      finalizeDaemonLeases({
+        leaseRegistry,
+        expiredProviderLeaseReleaser,
+        timeoutMs: 1_000,
+      }),
+    );
+
+    expect(release).not.toHaveBeenCalled();
+    // The daemon stops either way, so the registry entry goes; the summary and the log say this daemon
+    // never handed the device to its provider.
+    expect(outcomes.refused).toEqual([lease.leaseId]);
+    expect(leaseRegistry.listActiveLeases()).toEqual([]);
+    const rows = fs
+      .readFileSync(logPath, 'utf8')
+      .trim()
+      .split('\n')
+      .map((line) => JSON.parse(line) as { phase: string; data?: Record<string, unknown> });
+    expect(rows).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          level: 'error',
+          phase: 'provider_lease_release_refused',
+          data: {
+            leaseId: lease.leaseId,
+            reason: 'closed',
+            durable: false,
+            leaseProvider: 'browserstack',
+          },
+        }),
+      ]),
+    );
+  } finally {
+    fs.rmSync(stateDir, { recursive: true, force: true });
+  }
+});
+
 test('final drain joins a release that completes after the lease timeout', async () => {
   vi.useFakeTimers();
   const leaseRegistry = new LeaseRegistry();

packages/daemon/src/server/http/daemon-lease-finalizer.ts

@@ -1,60 +1,104 @@
 import { emitDiagnostic } from '@agent-device/host-kit/diagnostics';
-import type { ExpiredProviderLeaseReleaser } from '../provider-lease-expiry.ts';
+import type { DeviceLease } from '@agent-device/contracts/device';
+import {
+  leaseReleaseDiagnosticEvent,
+  type ExpiredProviderLeaseReleaser,
+  type ProviderLeaseReleaseOutcome,
+} from '../provider-lease-expiry.ts';
 import type { LeaseRegistry } from '../../resources/lease-registry.ts';
 import { leaseReleaseRequestFor } from '../../resources/lease-registry-scope.ts';
 
+/**
+ * The leases this finalizer was refused, by id. The shutdown summary reports these: a release its
+ * provider never saw is not a release this daemon completed.
+ */
+export type DaemonLeaseReleaseSummary = Readonly<{
+  refused: readonly string[];
+}>;
+
+/** What one lease came to here. The lease travels with it: the report must name it whichever way this went. */
+type LeaseReleaseSettlement =
+  | { lease: DeviceLease; outcome: ProviderLeaseReleaseOutcome }
+  | { lease: DeviceLease; timedOut: true }
+  | { lease: DeviceLease; error: unknown };
+
 /**
  * Ends every lease still active at daemon shutdown, whether a session held it or its owner kept it
  * through `close` with `retainOnClose`. Each provider release is bounded by `timeoutMs`; one that
- * runs past it stays with the releaser for the shutdown drain.
+ * runs past it stays with the releaser for the shutdown drain. A release the closed releaser refused is
+ * reported per lease and named in the returned summary, so the shutdown says the device was left with
+ * the provider instead of reading as a completed release.
  */
 export async function finalizeDaemonLeases(params: {
   leaseRegistry: LeaseRegistry;
   expiredProviderLeaseReleaser: ExpiredProviderLeaseReleaser;
   timeoutMs: number;
-}): Promise<void> {
+}): Promise<DaemonLeaseReleaseSummary> {
   const { leaseRegistry, expiredProviderLeaseReleaser, timeoutMs } = params;
-  await Promise.all(
-    leaseRegistry.listActiveLeases().map(async (lease) => {
+  const settlements = await Promise.all(
+    leaseRegistry.listActiveLeases().map(async (lease): Promise<LeaseReleaseSettlement> => {
       try {
-        const completed = await releaseWithinTimeout(
+        const outcome = await releaseWithinTimeout(
           expiredProviderLeaseReleaser.release(lease),
           timeoutMs,
         );
+        // The daemon stops either way, so the registry entry goes with it. What carries a lease past this
+        // process is its recovery record and its diagnostic, never this registry.
         leaseRegistry.releaseLease(leaseReleaseRequestFor(lease));
-        if (!completed) {
-          emitDiagnostic({
-            level: 'warn',
-            phase: 'daemon_shutdown_lease_release_timed_out',
-            data: { leaseId: lease.leaseId, timeoutMs },
-          });
-        }
+        if (outcome === undefined) return { lease, timedOut: true };
+        return { lease, outcome };
       } catch (error) {
-        emitDiagnostic({
-          level: 'warn',
-          phase: 'daemon_shutdown_lease_release_failed',
-          data: {
-            leaseId: lease.leaseId,
-            error: error instanceof Error ? error.message : String(error),
-          },
-        });
+        return { lease, error };
       }
     }),
   );
+  const refused: string[] = [];
+  for (const settlement of settlements) {
+    const { leaseId } = settlement.lease;
+    // A release whose budget ran out is reported here and answered by the drain, which owns the attempt.
+    if ('timedOut' in settlement) {
+      emitDiagnostic({
+        level: 'warn',
+        phase: 'daemon_shutdown_lease_release_timed_out',
+        data: { leaseId, timeoutMs },
+      });
+      continue;
+    }
+    if ('error' in settlement) {
+      emitDiagnostic({
+        level: 'warn',
+        phase: 'daemon_shutdown_lease_release_failed',
+        data: {
+          leaseId,
+          error:
+            settlement.error instanceof Error ? settlement.error.message : String(settlement.error),
+        },
+      });
+      continue;
+    }
+    // The mapping owns the event, so a refusal reads the same here as it does on the expiry route.
+    const event = leaseReleaseDiagnosticEvent(settlement.lease, settlement.outcome);
+    if (event) emitDiagnostic(event);
+    if (settlement.outcome.status === 'refused') refused.push(leaseId);
+  }
+  return Object.freeze({ refused: Object.freeze(refused) });
 }
 
-async function releaseWithinTimeout(release: Promise<void>, timeoutMs: number): Promise<boolean> {
-  return await new Promise<boolean>((resolve, reject) => {
-    const timeout = setTimeout(() => resolve(false), timeoutMs);
+async function releaseWithinTimeout<T>(
+  release: Promise<T>,
+  timeoutMs: number,
+): Promise<T | undefined> {
+  return await new Promise<T | undefined>((resolve, reject) => {
+    const timeout = setTimeout(() => resolve(undefined), timeoutMs);
     void release.then(
-      () => {
+      (value) => {
         clearTimeout(timeout);
-        resolve(true);
+        resolve(value);
       },
       (error: unknown) => {
         clearTimeout(timeout);
         reject(error);
       },
     );
   });
 }