From 887243c42afeeb011995ad4776cb554f195bfc24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 13:08:12 +0200 Subject: [PATCH 1/2] fix(daemon): keep an idle daemon alive only for retained leases Idle reap now waits only while an unexpired retainOnClose lease is active, through LeaseRegistry.hasRetainedLeases, which also expires leases past their window. Other leases, including human-control holds with no expiry, reap as before; ADR 0007 says so. A lifecycle-shutdown test restores the coverage that a session lease is released when its session teardown rejects. --- docs/adr/0007-remote-device-leases.md | 5 +-- src/daemon/__tests__/lease-registry.test.ts | 14 ++++++++ src/daemon/lease-registry.ts | 8 +++++ src/daemon/server/daemon-idle-reap.test.ts | 20 +++++------ src/daemon/server/daemon-idle-reap.ts | 9 ++--- .../daemon-runtime-lifecycle-shutdown.test.ts | 33 +++++++++++++++++++ src/daemon/server/daemon-runtime.ts | 2 +- 7 files changed, 74 insertions(+), 17 deletions(-) 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..18b574f477 100644 --- a/src/daemon/__tests__/lease-registry.test.ts +++ b/src/daemon/__tests__/lease-registry.test.ts @@ -71,6 +71,20 @@ 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); + assert.deepEqual(registry.listActiveLeases(), []); +}); + test('heartbeatLease extends active lease and releaseLease is idempotent', () => { let now = 1_000; const registry = new LeaseRegistry({ diff --git a/src/daemon/lease-registry.ts b/src/daemon/lease-registry.ts index 1e905a498f..1c6cd54c1a 100644 --- a/src/daemon/lease-registry.ts +++ b/src/daemon/lease-registry.ts @@ -182,6 +182,14 @@ export class LeaseRegistry { assertLeaseScopeMatch(this.getActiveLease(scope.leaseId), scope); } + /** + * Whether an unexpired `retainOnClose` lease outlives its sessions. Reading it also expires leases + * past their window, so an abandoned retained lease still ends. + */ + hasRetainedLeases(): boolean { + return this.listActiveLeases().some((lease) => lease.retainOnClose === true); + } + 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-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(); }, From e0d5f4c2d33d9f237f67dc1209547a626f3fb1e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 5 Oct 2026 18:52:09 +0200 Subject: [PATCH 2/2] fix(daemon): bound a retained lease's idle hold by its own expiry hasRetainedLeases counts a retainOnClose lease only while its own expiresAt is in the future, so a human-control hold (or admitted work) that keeps a past-due lease registered cannot keep an idle daemon alive forever. Tests: a registry case with a hold on the retained lease's device, and a runtime case where a plain lease does not block self-reap but a retained one does. --- src/daemon/__tests__/lease-registry.test.ts | 17 ++++- src/daemon/lease-registry.ts | 10 ++- .../server/daemon-runtime-idle-reap.test.ts | 64 +++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/daemon/__tests__/lease-registry.test.ts b/src/daemon/__tests__/lease-registry.test.ts index 18b574f477..c859733a85 100644 --- a/src/daemon/__tests__/lease-registry.test.ts +++ b/src/daemon/__tests__/lease-registry.test.ts @@ -82,7 +82,6 @@ test('only an unexpired retainOnClose lease counts as retained', () => { now = 20_000; assert.equal(registry.hasRetainedLeases(), false); - assert.deepEqual(registry.listActiveLeases(), []); }); test('heartbeatLease extends active lease and releaseLease is idempotent', () => { @@ -424,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 1c6cd54c1a..cddd286159 100644 --- a/src/daemon/lease-registry.ts +++ b/src/daemon/lease-registry.ts @@ -183,11 +183,15 @@ export class LeaseRegistry { } /** - * Whether an unexpired `retainOnClose` lease outlives its sessions. Reading it also expires leases - * past their window, so an abandoned retained lease still ends. + * 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 { - return this.listActiveLeases().some((lease) => lease.retainOnClose === true); + const now = this.now(); + return this.listActiveLeases().some( + (lease) => lease.retainOnClose === true && lease.expiresAt > now, + ); } listActiveLeases(): DeviceLease[] { 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 }); + } +});