Skip to content

fix(daemon): end the session and clear its claim when its lease ends - #3480

Draft
V3RON wants to merge 1 commit into
mainfrom
fix/session-ends-with-lease
Draft

V3RON wants to merge 1 commit into
mainfrom
fix/session-ends-with-lease

Conversation

@V3RON

@V3RON V3RON commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

When a lease was released or expired, the session opened under it stayed alive and kept its device claim. Its owner could not close it, and the next tenant's open failed with DEVICE_IN_USE. Expiry was only acted on by a later request, which often found nothing.

Now, when a lease ends, the daemon tears down the session bound to it under that session's execution locks and clears its claim, even if teardown fails. This covers tenant lease_release, host DELETE /admin/leases/<id>, and expiry (onLeaseExpired, plus a 1 s sweep so no request is needed). In-flight work and human-control holds still keep a lease alive. close and retainOnClose are unchanged. Help text and ADR 0007 updated. 13 files.

Validation

Tested commit 77f8e43. pnpm check:affected --run: all runnable checks passed (incl. layering, fallow, 2211 related tests).

New tests fail without the fix and pass with it: release ends the session; host DELETE ends it; expiry ends it with no request (runtime timer); an unreachable device still loses its claim; release waits for a running command.

Live run on a private-set iOS simulator, two proxy tenants, local daemon:

A open → claims: 1;  B open → DEVICE_IN_USE
lease.release(A) → released:true, claims: 0, leased_session_ended LEASE_RELEASED
B open → Opened, claims: 1;  B close → claims: 0
A open 09:58:00Z; no requests; 10:03:00.9Z leased_session_ended LEASE_EXPIRED, claims: 0
B open → Opened;  B close → claims: 0

Existing daemon and server suites pass (2010 tests). CI pending.

View guided diff Turn on auto-fix

Releasing a lease (tenant lease_release or host DELETE) or letting it
expire left the session opened under it holding its device claim, so the
next tenant's open failed with DEVICE_IN_USE and the owner could no longer
close it. The daemon now tears that session down under its execution locks
and clears the claim even when teardown fails, and sweeps the lease registry
every second so expiry no longer waits for a later request.
@thymikee

Copy link
Copy Markdown
Member

Thanks for this. The fix for ending a session and clearing its claim when the lease ends looks right at 77f8e43, but I found a shutdown race that needs fixing first. The five checks on this commit pass. The PR has merge conflicts with main, because #3431 rewrote daemon-runtime.ts, which holds the new wiring, so CI on the rebased head is the result that counts.

endSessionsForLease checks shuttingDown once, before it queues on the session locks (daemon-runtime.ts#L557). The locked task never checks again. A lease end started by the sweep or the registry is fired with void and is not in inFlightRequests, so shutdown() neither waits for it nor fences it. If SIGTERM, the shutdown command or a fatal error lands while that task runs or waits for the locks, settleIdleExpiredSession can run alongside detachForDaemonShutdown and teardownDaemonSessions on the same session. The runner meant for hand-off can be stopped, the device claim can be cleared twice, and shutdownClaimLedger can record the claim inconsistently. The rule is that no lease-end teardown starts after shutdown begins, and shutdown waits for any that already started. Re-check shuttingDown inside the withSessionLocks task, and track pending lease ends in a set that shutdown awaits before teardownDaemonSessions, as it does for inFlightRequests. Idle expiry already does this with closing() inside the lock pair and sessionIdleExpiry.cancel().

Not blocking, and you can take or leave these: endLeasedSession awaits settleIdleExpiredSession with no time limit, so a teardown that hangs delays the lease_release and host DELETE replies and never reaches clearDeviceClaim, while idle expiry bounds the same settle; the "waits for the command still running" test copies the withSessionLocks lambda from daemon-runtime.ts, so no test covers the shipped lock keys; and expiry now has two teardown routes (cleanupExpiredLeasedSession on the request path, settleIdleExpiredSession on the sweep path), so one function would be easier to reason about.

Simplicity question: could LeaseRegistry fire one onLeaseEnded(lease, reason) event from releaseLease and both consumeExpired* methods, wired once in daemon-runtime? That would drop the endSessionsForLease threading through router deps, the handler chain, the lease handler, the http server and host-lease-http. If endLeaseSessions took executionLocks and a closing() predicate and took the lock pair itself, as settleIdleSessionUnderLock does, it would also fix the race above in one place.

I did not run the tests or reproduce the live run, and I relied on your PR body output for it. I found the shutdown window by reading the code. Please rebase onto main first, then fence lease-end teardown against shutdown and rerun CI on the new head. I did not check how the change rebases onto #3431 or #3430.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants