diff --git a/docs/adr/0007-remote-device-leases.md b/docs/adr/0007-remote-device-leases.md index 70f6c87c10..e0edcacc3e 100644 --- a/docs/adr/0007-remote-device-leases.md +++ b/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 diff --git a/src/daemon/__tests__/lease-registry.test.ts b/src/daemon/__tests__/lease-registry.test.ts index 5b347e5781..c859733a85 100644 --- a/src/daemon/__tests__/lease-registry.test.ts +++ b/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 }); diff --git a/src/daemon/lease-registry.ts b/src/daemon/lease-registry.ts index 1e905a498f..cddd286159 100644 --- a/src/daemon/lease-registry.ts +++ b/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 })); diff --git a/src/daemon/server/daemon-idle-reap.test.ts b/src/daemon/server/daemon-idle-reap.test.ts index a907553fa1..9e7606dfb5 100644 --- a/src/daemon/server/daemon-idle-reap.test.ts +++ b/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++; }, diff --git a/src/daemon/server/daemon-idle-reap.ts b/src/daemon/server/daemon-idle-reap.ts index 8526c33164..2b7b9afbd5 100644 --- a/src/daemon/server/daemon-idle-reap.ts +++ b/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; } diff --git a/src/daemon/server/daemon-runtime-idle-reap.test.ts b/src/daemon/server/daemon-runtime-idle-reap.test.ts index 314fce795c..83f9580770 100644 --- a/src/daemon/server/daemon-runtime-idle-reap.test.ts +++ b/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(); + class RecordedLeaseRegistry extends actual.LeaseRegistry { + constructor(...args: ConstructorParameters) { + 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((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 }); + } +}); diff --git a/src/daemon/server/daemon-runtime-lifecycle-shutdown.test.ts b/src/daemon/server/daemon-runtime-lifecycle-shutdown.test.ts index ac90fcc75d..4594591de0 100644 --- a/src/daemon/server/daemon-runtime-lifecycle-shutdown.test.ts +++ b/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 { diff --git a/src/daemon/server/daemon-runtime.ts b/src/daemon/server/daemon-runtime.ts index 2513f2a992..43a7d1aad7 100644 --- a/src/daemon/server/daemon-runtime.ts +++ b/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(); },