feat(daemon): keep a caller-owned lease through session close with retainOnClose

[jbroma]

Summary

Session close always releases the session's lease. That has been the rule since #890, so a failed close can't strand a lease, and the CLI proxy flow relies on it to free devices. tester-army/e2e later started allocating leases itself and releasing them at run end (tester-army/e2e#416). It also closes sessions mid-run, on device.closeApp() and when it replaces a worker after a failing test file. Every later command fails UNAUTHORIZED: Lease is not active, and a provider-created Limrun instance gets deleted.

This adds an opt-in retainOnClose to leases.allocate. That lease survives close and ends through leases.release, expiry, or daemon shutdown, which now also releases leases no session holds. The default is unchanged, and an older daemon ignores the field.

await client.leases.allocate({ tenant, runId, leaseBackend: 'ios-instance', leaseProvider: 'limrun', retainOnClose: true });

22 files. Fixing this in e2e would cost it a fresh session after failures and leave closeApp broken. tester-army/e2e#792 covers local claims, unchanged here.

Validation

Tested commit 918e01b; CI pending on this head.

• pnpm check:affected --run: 4803 tests pass. Each new test fails under its mutation.
• Live Limrun iOS and Android, e2e unchanged, provider passing retainOnClose: closeApp then open, a failing file then the next, and a 75 s idle pass. Without the flag, it fails after closeApp.
• Org-key Android: the instance stays ready after close, terminated after daemon stop.

[jbroma]

This doesn't conflict with the #3116 session-lifetime stack (#3135 onward): it touches lease-lifecycle.ts and session-close.ts without changing the rule that close releases the lease, and it merges cleanly. Happy to rebase on top of it if you'd rather land the stack first.

[thymikee]

Thanks for the PR. I found no blocking problem in the code at 918e01b. Default close still releases the lease, and only a lease allocated with retainOnClose skips that step.

One piece of evidence is still missing. The PR body describes live Limrun iOS/Android runs (closeApp then open, a failing file then the next one, and the lease ending after a daemon stop), but there is no transcript. Could you paste the command output that shows the provider device still held after closeApp and released after the daemon stop?

Simplicity question: declaring retention on the lease at allocation looks like the right owner. But finalizeDaemonLeases already releases every lease left in the registry after teardown. Could the per-session finalizeDaemonSessionLease in beforeDelete go, leaving one shutdown sweep with one bounded release path? That would also cover sessions whose teardown never reached beforeDelete. What would have to change first? For example, does anything rely on the provider lease being released before the session record is deleted, such as the recoverable-lease journaling order or the session name in the diagnostic?

Not blocking, take or leave these:

• finalizeDaemonLeases is tested only by direct calls, so deleting its call in shutdown keeps every test green. One lifecycle-shutdown test with an unbound retainOnClose lease and a releaser would cover it.
• isDaemonIdle ignores active leases, so an unbound retained lease can be idle-reaped and released after 5 minutes even when its ttlMs is longer. Should ADR 0007 or the client-api docs say so, or should those leases count as activity?
• normalizeLease drops retainOnClose, so a caller cannot tell whether an older daemon honored it.
• Could lease reuse turn retainOnClose on for another client's lease through the cross-client branch? If so, upgrade only for the same clientId, or document it.
• The retain path skips the session_lease_released diagnostic; it could emit released: false, retained: true.

CI is green at this head, with 16 checks and none failing, and there are no conflicts. The PR is still a draft. Next step: the live transcript and an answer to the simplicity question.

[thymikee]

Finished the review items at 75c12eb, as commits on top of yours plus a merge of main. The new commits conflicted with main in daemon-runtime.ts, so I merged rather than rebased.

• Simplicity question: the per-session finalizeDaemonSessionLease and its beforeDelete hook are gone. Shutdown has one bounded release path, finalizeDaemonLeases, which runs after session teardown. Nothing relied on the earlier release:
  • Recoverable-lease journaling writes expired-provider-leases.json from the DeviceLease alone, never reading the session record. Its test passes unchanged.
  • The per-session shutdown diagnostic never reached a log: shutdown runs outside any diagnostics scope. The shutdown lease diagnostics are now daemon_shutdown_lease_release_timed_out / _failed and reach daemon.log through the shutdown scope.
• finalizeDaemonLeases test: a lifecycle-shutdown test starts the real runtime, allocates an unheld retainOnClose lease, and checks that shutdown releases it. It fails with the call removed.
• Idle reap: active leases now count as activity. The reaper re-arms instead of reaping, and the same registry read expires leases past their ttl, so an abandoned lease still ends. ADR 0007 and client-api.md say so.
• normalizeLease: it keeps retainOnClose: true only when the daemon honored it, so a caller can tell when an older daemon ignored it.
• Cross-client reuse: only the same clientId can turn retainOnClose on; the cross-client branch leaves the lease unchanged. A test covers it.
• Retain diagnostic: the retain path emits session_lease_released with released: false, retained: true.

Live run: Limrun iOS and Android, retain and default

Setup: `pnpm install --frozen-lockfile && pnpm build` in the worktree, then built again after the code commits. Each run starts its own daemon with an isolated state dir under <worktree>/.live-validation/state-<platform>-<mode>; the client starts it with AGENT_DEVICE_STATE_DIR set to that dir.

How credentials and instance specs were handled:
- The sandbox refused `set -a && . .env` (sourcing is not allowed in this harness). I used `node --env-file=<main>/.env` instead. The script deletes the AWS_/BROWSERSTACK_/OPENAI_/AI_GATEWAY_/EXA_ variables before the client starts the daemon, so the daemon gets only LIMRUN_API_KEY. The key was never printed.
- On this path the daemon's Limrun provider creates the instance at leases.allocate, so I could not create the instances myself with the limits you required. Instead, the script sets NODE_OPTIONS=--import limrun-create-guard.mjs for the daemon it starts. That preload wraps globalThis.fetch so that every POST /v1/{ios,android}_instances adds spec.hardTimeout '20m' and the label purpose=jbroma-pr-validation; the Limrun API output below confirms both on every instance.
- The script (.live-validation/retain-on-close.mjs) imports createAgentDeviceClient from the worktree's dist/src/index.js (the package "." export) and @limrun/api from the worktree's node_modules. Client config: {stateDir, session, tenant:'jbroma-pr', runId, clientId:'pr3208-validation', leaseBackend, leaseProvider:'limrun'}.
- Steps: client.leases.allocate({...scope, ttlMs: 600000, retainOnClose: true | omitted}), client.apps.open({app, platform, leaseId}), client.sessions.close({leaseId}). For retain it opens and closes a second time on the same lease, then runs `node bin/agent-device.mjs daemon stop --state-dir <dir>`. Instance state comes from the Limrun API (instances.list with labelSelector leaseId=<leaseId>).

$ node --env-file=<main>/.env .live-validation/retain-on-close.mjs ios retain
09:29:34 leases.allocate -> {"leaseId":"f3673423bef456cbcdc6fe45230f9ef8","retainOnClose":true}
09:29:34 limrun after allocate: ios_euna_01m45pb9kwe2yasmd748qpx8gc state=ready purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:34 session 1: apps.open(com.apple.Preferences) -> {"device":"limrun:ios:f3673423bef456cbcdc6fe45230f9ef8"}
09:29:34 session 1: sessions.close() -> ok
09:29:34 limrun after close: ios_euna_01m45pb9kwe2yasmd748qpx8gc state=ready purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:36 session 2 (same lease): apps.open(com.apple.Preferences) -> {"device":"limrun:ios:f3673423bef456cbcdc6fe45230f9ef8"}
09:29:36 session 2 (same lease): sessions.close() -> ok
09:29:36 limrun after second close: ios_euna_01m45pb9kwe2yasmd748qpx8gc state=ready purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:36 $ node bin/agent-device.mjs daemon stop --state-dir <worktree>/.live-validation/state-ios-retain
09:29:37 Daemon stopped (graceful).
09:29:37 limrun after daemon stop: ios_euna_01m45pb9kwe2yasmd748qpx8gc state=terminated purpose=jbroma-pr-validation hardTimeout=20m0s
state-ios-retain/daemon-shutdown.json: {"providerReleases":{"released":[{"leaseId":"f3673423bef456cbcdc6fe45230f9ef8","provider":"limrun"}],"pending":[]},"claims":{"released":[],"orphaned":[],"superseded":[],"unattributable":[]}}

$ node --env-file=<main>/.env .live-validation/retain-on-close.mjs ios default
09:29:44 leases.allocate -> {"leaseId":"2cedca204e1c9b76bda8f4deba4404e2"}
09:29:44 limrun after allocate: ios_euna_01m45pbk9re2y9mmwpra75sne0 state=ready purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:44 session 1: apps.open(com.apple.Preferences) -> {"device":"limrun:ios:2cedca204e1c9b76bda8f4deba4404e2"}
09:29:44 session 1: sessions.close() -> ok
09:29:44 limrun after close: ios_euna_01m45pbk9re2y9mmwpra75sne0 state=terminated purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:44 $ node bin/agent-device.mjs daemon stop --state-dir <worktree>/.live-validation/state-ios-default
09:29:44 Daemon stopped (graceful).

$ node --env-file=<main>/.env .live-validation/retain-on-close.mjs android retain
09:29:46 leases.allocate -> {"leaseId":"4baf8997b662c88668b12f54e76988da","retainOnClose":true}
09:29:46 limrun after allocate: android_euna_01m45pbnnkegzrwb2v1s569qx6 state=ready purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:47 session 1: apps.open(com.android.settings) -> {"device":"limrun:android:4baf8997b662c88668b12f54e76988da"}
09:29:47 session 1: sessions.close() -> ok
09:29:47 limrun after close: android_euna_01m45pbnnkegzrwb2v1s569qx6 state=ready purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:47 session 2 (same lease): apps.open(com.android.settings) -> {"device":"limrun:android:4baf8997b662c88668b12f54e76988da"}
09:29:47 session 2 (same lease): sessions.close() -> ok
09:29:47 limrun after second close: android_euna_01m45pbnnkegzrwb2v1s569qx6 state=ready purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:47 $ node bin/agent-device.mjs daemon stop --state-dir <worktree>/.live-validation/state-android-retain
09:29:47 Daemon stopped (graceful).
09:29:47 limrun after daemon stop: android_euna_01m45pbnnkegzrwb2v1s569qx6 state=terminated purpose=jbroma-pr-validation hardTimeout=20m0s

$ node --env-file=<main>/.env .live-validation/retain-on-close.mjs android default
09:29:49 leases.allocate -> {"leaseId":"f8a0ffcfe1ec797dd2aba95bab4b863d"}
09:29:49 limrun after allocate: android_euna_01m45pbrc3e2y9kkyp50aqvs6w state=ready purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:49 session 1: apps.open(com.android.settings) -> {"device":"limrun:android:f8a0ffcfe1ec797dd2aba95bab4b863d"}
09:29:50 session 1: sessions.close() -> ok
09:29:50 limrun after close: android_euna_01m45pbrc3e2y9kkyp50aqvs6w state=terminated purpose=jbroma-pr-validation hardTimeout=20m0s
09:29:50 $ node bin/agent-device.mjs daemon stop --state-dir <worktree>/.live-validation/state-android-default
09:29:50 Daemon stopped (graceful).

Result: a retainOnClose lease kept its Limrun instance `ready` through two session closes, a second open on the same lease worked, and `daemon stop` ended it (`terminated`, and daemon-shutdown.json lists it under providerReleases.released). A default lease's instance was `terminated` at session close.

Limits of this run:
- These runs drive the client directly. They do not run tester-army/e2e's closeApp path or its failing-file path.
- The 5-minute idle-reap behavior was not exercised live; unit tests cover it.

Decisions for you:

• Idle reap now counts every unexpired lease, not only retainOnClose ones. That includes a human-control hold with no expiry, which then keeps the daemon alive until the hold is removed. Keep that, or count only retained leases?
• The two shutdown diagnostic phases are renamed (see above). Nothing in the repo reads the old names.
• agent-device can't set hardTimeout or extra labels on instances it creates. The live run added them through a fetch preload. Should it have an option for that?

Left open from the independent review (minor): no test pins the daemon's hasActiveLeases wiring, and the new shutdown block adds a fifth copy of the withDiagnosticsScope(...daemon...) shape.

Gate: everything passes except one remote-proxy-parity test, which also fails on main on macOS. iOS open in that harness starts a real xcodebuild runner prewarm, and lease-expiry teardown then spawns pkill, which the hermetic signal guard refuses. Linux CI doesn't hit it, and it isn't caused by this PR.

[thymikee]

Reviewed at 75c12eb: the code looks good. A retainOnClose lease now survives session close, shutdown has one bounded release path, and the Limrun transcript above closes the earlier evidence gap. Checks pass and there are no conflicts.

One decision for you: hasActiveLeases in src/daemon/server/daemon-runtime.ts counts every lease, not only retainOnClose ones. So a human-control hold with no expiresAt now keeps an idle daemon alive, where before it was reaped. Is that intended? If yes, one sentence in ADR 0007 would record it; if not, count only retainOnClose leases.

Not blocking: the removed beforeDelete assertions were the only check that a session lease is released when its shutdown teardown rejects. One session-bound lease with a rejecting teardown in the lifecycle-shutdown test would restore that coverage. The idle-reap path that ends in onLeaseExpired is covered by tests only, not by a live run.

[thymikee]

On the open idle-reap question: this merged with every lease counting as activity. I chose to count only retainOnClose leases, the smaller behavior change: a human-control hold with no expiry reaps as before. That's in #3227, together with LeaseRegistry.hasRetainedLeases(), an ADR 0007 sentence, and the shutdown test for a session lease whose teardown rejects.