[thymikee]

Summary

A daemon killed with kill -9 while it held a cloud provider lease forgot that lease. Live provider leases were kept only in memory, so the rented device stayed billed until the provider's idle timeout.

The lease registry now reports its provider leases whenever one is issued or leaves. The daemon journals the ones that can be recovered in live-provider-leases.json. A lease that leaves for expiry stays in the journal until its expired-lease record is written, and a failed journal write is retried. At startup, any lease in the journal moves into the existing expired-lease recovery, which ends the device. ADR 0007 records the decision and its one gap. This touches 6 files and unblocks the kill -9 evidence on #4092.

Validation

• Tested at 8fd6e2d (base 65ec68b): pnpm check:affected --run passed all runnable checks.
• New tests in provider-lease-expiry.test.ts:
  • A killed daemon's lease is recovered exactly once.
  • A released lease is not recovered.
  • An expired lease stays journaled until its expired record is durable.
  • A failed journal write is retried.
  • Both mutation-checked tests fail without their fix.
• Live on Revyl, with this commit applied to feat(revyl): add the Revyl device-cloud provider plugin #4092: the daemon was killed at 18:25:49, and Revyl recorded the session completed at 18:25:51.
• Known gap: the registry releases a canceled first allocation without asking the provider. So that lease leaves the journal while its start may still be running, and a crash in that window loses it.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
agent-device-website	Ready	Preview	Oct 30, 2026 10:43pm UTC

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.61 MB	5.61 MB	+1.7 kB
Package (unpacked)	5.61 MB	5.61 MB	+1.7 kB
Package (download)	1.71 MB	1.71 MB	+440 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	43.7 ms	41.4 ms	-2.4 ms
CLI --help	86.3 ms	84.4 ms	-2.0 ms

[cubic-dev-ai]

All reported issues were addressed across 6 files

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[thymikee]

Reviewed 9850f4c. The recovery design is sound, but a refused daemon start can release devices that a live daemon still uses, so this needs one change before merge.

daemon-runtime.ts:368: createExpiredProviderLeaseReleaser adopts orphaned leases in its constructor (provider-lease-expiry.ts:115), and line 376 then runs void retryPending(). Both run before tryAcquireDaemonRegistration at line 614. Suppose a second daemon starts for a state dir that a live daemon owns, and line 614 later refuses it with daemon_registration_refused. That process has already read the live daemon's live-provider-leases.json, moved its leases into expired-provider-leases.json, and deleted the journal. It then calls recoverExpiredLease, which for Limrun is release(lease), on leases that the live daemon's sessions still use. Before this change, this pre-lock step touched only already-expired leases. Now a refused or racing start can end rented devices under running sessions, and it strips the live daemon's crash journal until that daemon's next lease change. The rule: only the process that holds the registration lock of a state dir reads, deletes or acts on that state dir's lease records. Please move adoption out of the constructor into an explicit releaser step, or build the releaser after registration. Call that step and the startup retryPending only after tryAcquireDaemonRegistration returns acquired, next to recoverAppLogResourcesAfterDaemonLock and recoverStartupResources. Please add a test where a refused start leaves a live journal and its leases untouched.

Not blocking, and you can take or leave these: (1) recordLiveLeases writes only the registry snapshot, so when persistPendingLeases fails (for example ENOSPC at line 226) the journal rmSync can still succeed and the lease ends up in neither file. The invariant is that every unreleased recoverable lease is in the pending record or the live journal, so recordLiveLeases could write the snapshot plus every pendingRecoveryLeases entry not in persistedRecoveryLeaseIds. (2) ADR 0007:88 records journaling but not the two routes where the registry forgets a device without a crash: lease.ts:278 (failed first allocation) and lease.ts:305 (a provider release that throws for a gone requester still releases the registry entry). One sentence naming both as known gaps would cover it. (3) writeLeaseRecord and loadLeaseRecord now serve both files but keep the PersistedExpiredProviderLeases type and PENDING_RELEASES_VERSION, so a name like PersistedProviderLeaseRecord fits better.

The open inline threads still apply: the persist-failure drop from the live journal (#4108 (comment), P1), the swallowed journal write failure (#4108 (comment), P1, which I see as a policy call and non-blocking), the ADR gap list (#4108 (comment), P2), and the rmSync throw during startup (#4108 (comment), P2). One more P2 (#4108 (comment), no retry on the journal write) is low impact because the next lease change rewrites the snapshot. The ADR wording thread (#4108 (comment), P3) also applies.

CI is green: 18 checks, none failing. There are no conflicts.

I did not run the code or the new tests. I could not verify the Revyl kill -9 live run, and no live run on Limrun is reported. I also did not check whether Limrun's release is safe to repeat for an instance that is already gone. If a provider throws on a second recovery, that lease would retry every second. I confirmed the startup ordering in the code but did not trace how often CLI auto-start produces a refused second daemon.

Before merge, please move orphan adoption and the startup retry after tryAcquireDaemonRegistration, and add the refused-start test.

[cubic-dev-ai]

All reported issues were addressed across 3 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[apex-sentinel-staging]

Apex Sentinel

Verdict: approve

Since the last review: 8 new · 0 still open · 6 no longer raised

8 finding(s): 1 minor, 7 nit. Verdict: approve. reached max rounds (1).

🟡 Minor (1)

Where	Finding
packages/daemon/src/server/provider-lease-expiry.ts:417	🆕 recoverInherited() drops a lease record whose version or shape it cannot read, deleting the only crash recovery for those leases

🔵 Nit (7)

Where	Finding
packages/daemon/src/resources/lease-registry.ts:183	🆕 reportProviderLeases skips the journal update whenever the changed lease carries no leaseProvider, leaving the stale-journal invariant implicit
packages/daemon/src/resources/lease-registry.ts:594	🆕 Journal is rewritten once per departing lease inside the expiry sweep loop
packages/daemon/src/server/provider-lease-expiry.ts:51	🆕 retryPending is now dead production code but stays in the public releaser type
packages/daemon/src/server/provider-lease-expiry.ts:128	🆕 recordLiveLeases writes the live journal before recoverInherited, clobbering the journal it is meant to adopt
packages/daemon/src/server/provider-lease-expiry.ts:166	🆕 recoverInherited marks itself done before its work can fail, so a failed adoption is never retried in-process
packages/daemon/src/server/provider-lease-expiry.ts:179	🆕 Journal copy of a lease keeps its stale expiresAt, so a killed daemon's journal can look live to the next one forever
packages/daemon/src/server/provider-lease-expiry.ts:316	🆕 shutdown clears journalRetryTimer twice and its last-write comment overstates what it does

Reply under an inline comment with @apex-sentinel-staging note ... or @apex-sentinel-staging pivot ... to re-check that finding.
Comment @apex-sentinel-staging review on this conversation to run a full review.

[apex-sentinel-staging]

Apex Sentinel: 8 new findings on 8fd6e2d. The verdict and the full list are in the summary comment.

[thymikee]

Thanks for the update. At 290a0be the journal handoff gaps from the earlier review are fixed, but the startup ordering problem is still there.

A daemon that does not hold the state dir lock can still act on that dir's lease records. createExpiredProviderLeaseReleaser adopts the journal in its constructor (provider-lease-expiry.ts:122), and daemon-runtime.ts:376 then calls retryPending(). Both run before tryAcquireDaemonRegistration at :614. Suppose a second daemon starts on a state dir that a live daemon owns, for example a racing CLI auto-start or an embedded SDK runtime. It reads that daemon's live-provider-leases.json, rewrites expired-provider-leases.json, deletes the journal, and releases those leases at the provider. Only then does :615 refuse it with daemon_registration_refused. A refused start can therefore end rented devices that a running daemon's sessions still use. It also strips that daemon's crash journal, so a kill -9 in that window loses the leases this PR exists to recover. The rule to satisfy: only the process that holds the registration lock may read, rewrite, delete or act on that state dir's lease records. Please keep the constructor free of disk work. Move loading the pending record, adopting the journal and the startup retryPending() into one releaser step, for example recoverInherited(), and call it only after acquisition.status === 'acquired', next to recoverAppLogResourcesAfterDaemonLock (:669) and recoverStartupResources (:687). Please add a test where registration is refused, live-provider-leases.json is byte-identical afterward and recoverExpiredLease is not called. That test should fail without the move.

Not blocking, and you can take or leave it: the journal is rewritten only by recordLiveLeases, but persistPendingLeases (:84-96) and the delete at :217 also change what it should contain, and shutdown() (:288-289) cancels a pending journal retry without a final write, so after a failed first persist the journal can keep naming a lease that was already recovered until the next provider-lease change; calling writeJournal() at the end of a successful persistPendingLeases and flushing once in shutdown() when journalRetryTimer is set would keep every change to liveLeases, pendingRecoveryLeases and persistedRecoveryLeaseIds reflected in the journal.

A question, optional: this delta guards a handoff between two files (the unpersisted merge, persistedRecoveryLeaseIds, a second retry timer and the remove-failure catch). Would one record per state dir be simpler? It would hold every unreleased recoverable lease, live and expiring, be written by one function from liveLeases and pendingRecoveryLeases with one retry, and be loaded and recovered once after the lock. That removes the move-then-delete adoption and the transfer window. The f1 fix already reshapes startup loading, so now is the cheapest time. It would need ADR 0007:89-94 amended to name a single record, and maintainer approval on whether a daemon reads an existing expired-provider-leases.json once after upgrade, since skipping that read drops pending expired leases.

On the open inline threads: no P1 or P2 thread still applies, and six lower-priority threads still apply (the startup-order one is covered by the finding above). These do not apply and can be resolved: r4428007864 because a provider lease cannot become a non-provider lease in place (lease-registry-scope.ts:316); r4428007870 because the recoverable filter belongs in the releaser and the file holds only recoverable leases; r4427531525, r4427531533, r4427531540, r4427531559, r4427531577, r4427531586 and r4427865994 because the code, ADR text or tests at this head now cover them.

Coverage and Repo Guards & Compatibility pass at 290a0be, and two Smoke Tests jobs pass. One Smoke Tests job is still pending; nothing has failed. The daemon startup route that smoke tests boot through is in the diff, so a failure there would need attribution.

This change touches daemon startup for provider leases, so it needs one Revyl run at the fixed head. The run should start a daemon, allocate a revyl lease, kill -9 the daemon, and start a new daemon on the same state dir. The new daemon's log must show lock-acquired before provider_lease_orphans_adopted with that leaseId, and Revyl must record the session as completed. A second daemon refused on a live state dir must leave live-provider-leases.json and the live daemon's device untouched. The earlier kill -9 claim does not name its commit, and I could not verify it. I did not run the code or the new tests, and I did not check whether Limrun or Revyl recovery is safe to repeat for a device that is already gone. Moving adoption behind the registration lock and adding the refused-start test is the next step before merge.

[apex-sentinel-staging]

Apex Sentinel: 6 new findings on bc75a53. The verdict and the full list are in the summary comment.

[apex-sentinel-staging]

Apex Sentinel: 6 new findings on e23e0b5. The verdict and the full list are in the summary comment.

[cubic-dev-ai]

All reported issues were addressed across 4 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[apex-sentinel-staging]

Apex Sentinel: 5 new findings on 9c93b9e. The verdict and the full list are in the summary comment.

[cubic-dev-ai]

All reported issues were addressed across 3 files (changes from recent commits).

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[thymikee]

Thanks. The ordering you described was the real defect, and the latest commits fix it:

• 665e27c, 79692a9: the releaser does no disk work at construction. One recoverInherited() step loads the expired record, adopts the journal, and starts retryPending(). Startup calls it after the registration is acquired, next to recoverAppLogResourcesAfterDaemonLock.
• 9c93b9e: adoption is reported through the startup diagnostics flushed to daemon.log. Startup has no request scope, so emitDiagnostic was dropping that report.
• 1641aa5: src/host.test.ts starts a second host on a held state directory. After the refusal, the journal is byte-identical and no expired record exists. The test fails if recoverInherited moves before the registration.
• c8a7626: a failure inside recoverInherited is reported (provider_lease_startup_recovery_failed), not left unhandled.

Also taken from your notes:

• A durable expired record rewrites the journal, and shutdown() flushes a journal write that is still owed.
• The record type is now PersistedProviderLeaseRecord, with one LEASE_RECORD_VERSION.
• ADR 0007 names both routes where the registry forgets a provider lease without a crash.

On the one-record design: I kept two files. One record would mean a migration that reads an existing expired-provider-leases.json once after an upgrade, and dropping that read loses pending expired leases. Since that needs your call on the upgrade read, I'd rather do it as its own change if you want it.

Live run against the real Revyl backend. Packed from #4092 at 86c4c7f with this PR's commits on top (through c8a7626), installed through a local registry. Android device, one live revyl lease, 40f23ed9…:

1. Refused second runtime. While daemon A (pid 49182) held the lease, startAgentDevice({ stateDir }) from the packed SDK answered daemon_registration_refused busy.
   • live-provider-leases.json had the same SHA-1 before and after (d21e7e9a…).
   • No expired-provider-leases.json appeared.
   • Revyl still listed the session running.
2. kill -9 and restart.
   • Daemon A was killed at 22:14:48.
   • session list started daemon B (pid 49265). Its daemon.lock/owner.json shows acquiredAtMs 22:14:48.708Z.
   • daemon.log shows provider_lease_orphans_adopted with leaseIds: ["40f23ed9709007df6d3569a35bc4be11"] at 22:14:48.731Z, 23 ms after the lock was taken.
   • Revyl history: session db7c5aea… completed, ended_at 22:14:51.92. Both lease files were gone afterwards.

The startup-phase marker file is deleted once a startup succeeds, so the lock's own acquiredAtMs is the lock-acquired evidence here.

[apex-sentinel-staging]

Apex Sentinel: 7 new findings on ecd2caf. The verdict and the full list are in the summary comment.

[apex-sentinel-staging]

Apex Sentinel could not review commit 95e6b335. No review ran, so there are no findings; this is not a verdict on the change.

Apex Sentinel could not read or check out this pull request, which is usually temporary (GitHub was slow, or the branch moved while the review waited). If it keeps failing, tell whoever runs Sentinel for this repository, or check that the App still has access to it. Push a new commit or comment @apex-sentinel-staging review to try again.

[thymikee]

The code looks good at 95e6b33. The earlier ordering problem is fixed: adoption now runs only after the daemon holds its registration, and src/host.test.ts:369 proves the refused-start route through the real startup. The earlier journal and shutdown findings are fixed too.

Not blocking, and fine to take or leave: an unreadable live-provider-leases.json is left in place at startup, but the next lease change or expiry rewrites it with only this daemon's leases, so the dead daemon's records are lost (provider-lease-expiry.ts:154 and :422; renaming the file aside before any write would keep the evidence, or the "left for an operator" comment can go); the test at provider-lease-expiry.test.ts:382 compares text positions in daemon-runtime.ts, which src/host.test.ts:369 already covers by behavior, so it can go; and retryPending in provider-lease-expiry.ts:51 has no production caller now, so it can leave the type and the tests can call recoverInherited.

The only failing check is the review-bot job, whose own comment says it could not read the PR, so it did not run on this code. The other 18 checks pass, including the jobs that start the daemon through the changed path. There are no conflicts.

I read the code and did not run it. The live Revyl kill -9 run is author-reported and covers commits through c8a7626. The two later commits change only failure-path diagnostics. I did not check that Limrun recovery is safe to repeat for a device that is already gone, or trace ADR 0007's second gap (late allocation whose provider release throws).

No open P1 or P2 thread still applies. Two lower-priority resolved threads still apply and match the notes above (#4108 (comment), #4108 (comment)). These threads are fixed at this commit and already resolved: journal now kept until the expired record lands (#4108 (comment)); adoption now runs after registration (#4108 (comment)); shutdown now flushes an owed journal write (#4108 (comment)); only recoverable providers are recorded (#4108 (comment)); record type and constant renamed (#4108 (comment)); failed persist now returns before the journal is removed (#4108 (comment)); retry timer re-runs the journal write (#4108 (comment)); merge order is now commented (#4108 (comment)); adoption sits behind the registration, and src/host.test.ts:369 covers refusal (#4108 (comment)); pending set is set before the first await, with a comment (#4108 (comment)); src/host.test.ts:369 now drives the real registration refusal (#4108 (comment)); recoverInherited now has a catch (#4108 (comment)); startup failure now goes through the daemon diagnostic (#4108 (comment)). These threads do not apply, for the reason given, and are already resolved: a provider lease cannot become a non-provider lease in place (#4108 (comment)); the stateDir guard narrows the type (#4108 (comment)); the sync write runs only on rare provider lease changes (#4108 (comment)); the remaining gap is stated in ADR 0007 (#4108 (comment)); same stateDir guard (#4108 (comment)); the second timer clear is deliberate (#4108 (comment)); adoption never reads expiresAt (#4108 (comment)); same sync-write point (#4108 (comment)); a lease in both files is deduped by leaseId (#4108 (comment)); record names are unified (#4108 (comment)); same stateDir guard (#4108 (comment)); same sync-write point (#4108 (comment)); recovery uses leaseId and provider, not expiresAt (#4108 (comment)); that loop marks leases durable without writing, so it is not a copy (#4108 (comment)); production calls only recoverInherited (#4108 (comment)); a provider lease cannot lose its provider in place (#4108 (comment)); one rewrite per departing lease is a small cost on a rare path (#4108 (comment)); no provider lease can change before the servers open (#4108 (comment)); a failed adoption persist is retried from the journal (#4108 (comment)); adoption ignores expiresAt (#4108 (comment)).