fix(daemon): keep an idle daemon alive only for retained leases

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

@@ -48,8 +48,9 @@ its provider device in place, and only `leases.release`, expiry, or daemon
 shutdown ends it. The default is unchanged so the CLI's proxy sharing still
 frees devices on `close`. The allocated lease reports `retainOnClose: true`
 only when the daemon honored it, and only the lease's own client can turn it
-on for a lease the run already holds. An unexpired lease keeps an idle daemon
-alive; one the caller stops heartbeating expires after its `ttlMs`, and idle
+on for a lease the run already holds. An unexpired `retainOnClose` lease keeps
+an idle daemon alive; other leases, including human-control holds, do not. A
+retained lease the caller stops heartbeating expires after its `ttlMs`, and idle
 reap then shuts the daemon down within one more idle window.
 
 ## Consequences

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

@@ -71,6 +71,19 @@ test('a request without the owning clientId cannot turn retainOnClose on for a r
   assert.equal(reused.retainOnClose, undefined);
 });
 
+test('only an unexpired retainOnClose lease counts as retained', () => {
+  let now = 1_000;
+  const registry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 10_000 });
+  registry.allocateLease({ tenantId: 'tenant-a', runId: 'run-plain' });
+  assert.equal(registry.hasRetainedLeases(), false);
+
+  registry.allocateLease({ tenantId: 'tenant-a', runId: 'run-retained', retainOnClose: true });
+  assert.equal(registry.hasRetainedLeases(), true);
+
+  now = 20_000;
+  assert.equal(registry.hasRetainedLeases(), false);
+});
+
 test('heartbeatLease extends active lease and releaseLease is idempotent', () => {
   let now = 1_000;
   const registry = new LeaseRegistry({
@@ -410,6 +423,22 @@ test('human holds protect leases and release refreshes the original lease TTL at
   assert.equal(registry.consumeExpiredLeases()[0]?.leaseId, lease.leaseId);
 });
 
+test('a human-control hold keeps a past-due retained lease registered but not retained', async () => {
+  let now = 1_000;
+  const registry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 5_000 });
+  const lease = registry.allocateLease({
+    ...HUMAN_CONTROL_LEASE_REQUEST,
+    ttlMs: 10_000,
+    retainOnClose: true,
+  });
+  await registry.putHumanControlHold({ kind: 'lease', leaseId: lease.leaseId }, 'console', {});
+  assert.equal(registry.hasRetainedLeases(), true);
+
+  now = 25_000;
+  assert.equal(registry.listActiveLeases()[0]?.leaseId, lease.leaseId);
+  assert.equal(registry.hasRetainedLeases(), false);
+});
+
 test('hold expiry refreshes from the expiry instant, without reviving abandoned leases', async () => {
   let now = 0;
   const registry = new LeaseRegistry({ now: () => now, defaultLeaseTtlMs: 5_000 });

src/daemon/lease-registry.ts

@@ -182,6 +182,18 @@ export class LeaseRegistry {
     assertLeaseScopeMatch(this.getActiveLease(scope.leaseId), scope);
   }
 
+  /**
+   * Whether a `retainOnClose` lease is still inside its own window. A human-control hold or admitted
+   * work can keep a past-due lease registered, but only the lease's `expiresAt` bounds how long it
+   * keeps an idle daemon alive.
+   */
+  hasRetainedLeases(): boolean {
+    const now = this.now();
+    return this.listActiveLeases().some(
+      (lease) => lease.retainOnClose === true && lease.expiresAt > now,
+    );
+  }
+
   listActiveLeases(): DeviceLease[] {
     this.cleanupExpiredLeases();
     return Array.from(this.leases.values()).map((entry) => ({ ...entry }));

src/daemon/server/daemon-idle-reap.test.ts

@@ -149,7 +149,7 @@ test('idle reap fires after the idle window when nothing is using the daemon', a
   const idleReap = createDaemonIdleReap({
     sessionStore,
     getInFlightRequestCount: () => 0,
-    hasActiveLeases: () => false,
+    hasRetainedLeases: () => false,
     onIdleReap: () => {
       reaped++;
     },
@@ -164,14 +164,14 @@ test('idle reap fires after the idle window when nothing is using the daemon', a
   assert.equal(reaped, 1);
 });
 
-test('idle reap waits another window while a lease no session holds is still active', async () => {
+test('idle reap waits another window while a retained lease is still active', async () => {
   vi.useFakeTimers();
   let reaped = 0;
-  let leaseActive = true;
+  let retainedLeaseActive = true;
   const idleReap = createDaemonIdleReap({
     sessionStore,
     getInFlightRequestCount: () => 0,
-    hasActiveLeases: () => leaseActive,
+    hasRetainedLeases: () => retainedLeaseActive,
     onIdleReap: () => {
       reaped++;
     },
@@ -181,7 +181,7 @@ test('idle reap waits another window while a lease no session holds is still act
   idleReap.noteActivity();
   await vi.advanceTimersByTimeAsync(120);
   assert.equal(reaped, 0);
-  leaseActive = false;
+  retainedLeaseActive = false;
   await vi.advanceTimersByTimeAsync(40);
 
   assert.equal(reaped, 1);
@@ -194,7 +194,7 @@ test('idle reap does not fire while a session is open', async () => {
   const idleReap = createDaemonIdleReap({
     sessionStore,
     getInFlightRequestCount: () => 0,
-    hasActiveLeases: () => false,
+    hasRetainedLeases: () => false,
     onIdleReap: () => {
       reaped++;
     },
@@ -223,7 +223,7 @@ test('idle reap does not fire while a recording is active', async () => {
   const idleReap = createDaemonIdleReap({
     sessionStore,
     getInFlightRequestCount: () => 0,
-    hasActiveLeases: () => false,
+    hasRetainedLeases: () => false,
     onIdleReap: () => {
       reaped++;
     },
@@ -243,7 +243,7 @@ test('idle reap does not fire while a request is in flight', async () => {
   const idleReap = createDaemonIdleReap({
     sessionStore,
     getInFlightRequestCount: () => inFlightRequestCount,
-    hasActiveLeases: () => false,
+    hasRetainedLeases: () => false,
     onIdleReap: () => {
       reaped++;
     },
@@ -266,7 +266,7 @@ test('idle reap is disabled when the window is zero', async () => {
   const idleReap = createDaemonIdleReap({
     sessionStore,
     getInFlightRequestCount: () => 0,
-    hasActiveLeases: () => false,
+    hasRetainedLeases: () => false,
     onIdleReap: () => {
       reaped++;
     },
@@ -285,7 +285,7 @@ test('cancel prevents a scheduled reap from firing', async () => {
   const idleReap = createDaemonIdleReap({
     sessionStore,
     getInFlightRequestCount: () => 0,
-    hasActiveLeases: () => false,
+    hasRetainedLeases: () => false,
     onIdleReap: () => {
       reaped++;
     },

src/daemon/server/daemon-idle-reap.ts

@@ -84,10 +84,11 @@ export function createDaemonIdleReap(params: {
   sessionStore: SessionStore;
   getInFlightRequestCount: () => number;
   /**
-   * An unexpired lease is a client's claim on this daemon, so a reap that finds one waits another
-   * idle window. The check must also expire leases past their window, or none would ever end.
+   * An unexpired `retainOnClose` lease is a caller's claim on this daemon beyond its sessions, so a
+   * reap that finds one waits another idle window. The check must also expire leases past their
+   * window, or none would ever end.
    */
-  hasActiveLeases: () => boolean;
+  hasRetainedLeases: () => boolean;
   onIdleReap: () => void;
   env?: NodeJS.ProcessEnv;
 }): DaemonIdleReapController {
@@ -115,7 +116,7 @@ export function createDaemonIdleReap(params: {
       // session open or new request must never lose a race against a
       // previously scheduled reap.
       if (!isIdleNow()) return;
-      if (params.hasActiveLeases()) {
+      if (params.hasRetainedLeases()) {
         schedule();
         return;
       }

src/daemon/server/daemon-runtime-idle-reap.test.ts

@@ -1,12 +1,28 @@
 import assert from 'node:assert/strict';
 import fs from 'node:fs';
 import { afterEach, test, vi } from 'vitest';
+
+const leaseProbe = vi.hoisted(() => ({
+  registries: [] as import('../lease-registry.ts').LeaseRegistry[],
+}));
+vi.mock('../lease-registry.ts', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../lease-registry.ts')>();
+  class RecordedLeaseRegistry extends actual.LeaseRegistry {
+    constructor(...args: ConstructorParameters<typeof actual.LeaseRegistry>) {
+      super(...args);
+      leaseProbe.registries.push(this);
+    }
+  }
+  return { ...actual, LeaseRegistry: RecordedLeaseRegistry };
+});
+
 import { resolveDaemonPaths } from '../../daemon-resolution.ts';
 import { startDaemonRuntime } from './daemon-runtime.ts';
 import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts';
 
 afterEach(() => {
   vi.useRealTimers();
+  leaseProbe.registries.length = 0;
 });
 
 test('daemon runtime self-reaps after the idle window when nothing ever uses it', async () => {
@@ -85,3 +101,51 @@ test('daemon runtime never self-reaps when AGENT_DEVICE_DAEMON_IDLE_TIMEOUT_MS i
     fs.rmSync(stateDir, { recursive: true, force: true });
   }
 });
+
+test('only a retained lease keeps the daemon runtime from self-reaping', async () => {
+  vi.useFakeTimers();
+  const stateDir = mkdtempForTestSync('agent-device-daemon-idle-reap-leases-');
+  let exitCode: number | undefined;
+  let resolveExit: () => void;
+  const exited = new Promise<void>((resolve) => {
+    resolveExit = resolve;
+  });
+
+  try {
+    const runtime = await startDaemonRuntime({
+      env: {
+        ...process.env,
+        AGENT_DEVICE_STATE_DIR: stateDir,
+        AGENT_DEVICE_DAEMON_SERVER_MODE: 'http',
+        AGENT_DEVICE_DAEMON_IDLE_TIMEOUT_MS: '80',
+      },
+      exit: (code) => {
+        exitCode = code;
+        resolveExit();
+      },
+      registerProcessHandlers: false,
+      stderr: { write: () => {} },
+      stdout: { write: () => {} },
+    });
+    assert.notEqual(runtime, null);
+    const [registry] = leaseProbe.registries;
+    registry!.allocateLease({ tenantId: 'tenant-a', runId: 'run-plain', ttlMs: 60_000 });
+    const retained = registry!.allocateLease({
+      tenantId: 'tenant-a',
+      runId: 'run-retained',
+      ttlMs: 60_000,
+      retainOnClose: true,
+    });
+
+    await vi.advanceTimersByTimeAsync(80);
+    assert.equal(exitCode, undefined);
+
+    registry!.releaseLease({ leaseId: retained.leaseId });
+    await vi.advanceTimersByTimeAsync(80);
+    await vi.runOnlyPendingTimersAsync();
+    await exited;
+    assert.equal(exitCode, 0);
+  } finally {
+    fs.rmSync(stateDir, { recursive: true, force: true });
+  }
+});

src/daemon/server/daemon-runtime-lifecycle-shutdown.test.ts

@@ -399,6 +399,39 @@ test.each([false, true])(
   },
 );
 
+test('daemon shutdown releases a session lease whose session teardown rejects', async () => {
+  const stateDir = mkdtempForTestSync('agent-device-daemon-session-lease-shutdown-');
+  const runtime = await startDaemonRuntime({
+    env: {
+      ...process.env,
+      AGENT_DEVICE_STATE_DIR: stateDir,
+      AGENT_DEVICE_DAEMON_IDLE_TIMEOUT_MS: '0',
+      AGENT_DEVICE_DAEMON_SERVER_MODE: 'http',
+    },
+    exit: () => {},
+    registerProcessHandlers: false,
+    stderr: { write: () => {} },
+    stdout: { write: () => {} },
+  });
+  const [leaseRegistry] = leaseProbe.registries;
+  const lease = leaseRegistry!.allocateLease({
+    tenantId: 'tenant-a',
+    runId: 'run-1',
+    leaseProvider: 'limrun',
+  });
+  shutdownProbe.store!.publish('bound', {
+    ...makeIosSession('bound'),
+    lease: { leaseId: lease.leaseId, tenantId: lease.tenantId, runId: lease.runId },
+  });
+  shutdownProbe.finalize.mockRejectedValueOnce(new Error('teardown failed'));
+
+  await runtime?.shutdown();
+
+  expect(shutdownProbe.finalize).toHaveBeenCalledOnce();
+  expect(leaseProbe.released).toEqual([lease]);
+  expect(leaseRegistry!.listActiveLeases()).toEqual([]);
+});
+
 test('daemon shutdown releases a retainOnClose lease that no session holds', async () => {
   const stateDir = mkdtempForTestSync('agent-device-daemon-retained-lease-shutdown-');
   try {

src/daemon/server/daemon-runtime.ts

@@ -579,7 +579,7 @@ export async function startDaemonRuntime(
   const idleReap = createDaemonIdleReap({
     sessionStore,
     getInFlightRequestCount: () => inFlightRequests.size,
-    hasActiveLeases: () => leaseRegistry.listActiveLeases().length > 0,
+    hasRetainedLeases: () => leaseRegistry.hasRetainedLeases(),
     onIdleReap: () => {
       void shutdown();
     },