[thymikee]

Summary

Revyl kept handing out devices after release started or failed. One lease owner now makes starting, ready, stopping, stop-failed and closed explicit. It retires admission synchronously, shares bounded disposal, retains uncertain cleanup for retry, and disposes late creates once.

Seven files; scope stays within Revyl lifecycle, adjacent tests and its guide. Related: #4102; follows #4130 and #4141. Eager closure is preserved.

Final review round derives disposal's stop budget from the HTTP client's full retry budget plus a margin, preserves typed retirement when a waiting caller gives up, and caches ready-session reuse. Field and callback-signature nits are explained in their threads.

Validation

Base a0e7c7dc239922bf93adaceff20a2f7e78579146; head a8ebe1f45f164249fc8d4a04221000ab1dd9d180; previous reviewed head 240b05012f6895ea90406d974c80102b94745903.

• Two new regressions fail at the reviewed head and pass here; all 149 Revyl tests pass. Earlier five lifecycle regressions fail against base sources. The early-release stop assertion detects a removed stop call; mutation was reverted.
• pnpm check:affected --run --head HEAD passed: receipt run-trpcM0, exactCommit=true, checkoutChanged=false; 16,972 unit tests passed, one skipped. Full-repository format passed.
• Previous live allocation returned HTTP 401 Invalid API key: one rejected create, zero sessions created. Command/release/listing were blocked. Host cleanup had no pending/unknown releases; private-state daemon stop reported known cleanup with no warnings. No new live run this round.
• CI Integration Tests and Coverage remain required at this head.

[vercel]

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

Project	Deployment	Actions	Updated
agent-device-website	Ready	Preview	Nov 2, 2026 10:43am UTC

[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]

Request changes.

• (a) Holds: allocate validates each caller before joining; rent publishes the owner before its start microtask, so joiners observe the same creation without inheriting caller validation errors.
• (b) Holds: release/shutdown call the same owner.end; a late create after the 120s budget re-enters end, records rent, and stops once. Evidence: lease-owner.test.ts pending-create release/shutdown case.
• (c) Holds: create and stop waits are bounded (120s/30s); uncertain stop remains stop-failed with the stop promise retained for retry. All disposal provider calls are behind start/readiness budgets or the bounded stop wait.
• (d) Holds: startDevice uses POST and Revyl client defaults retries only GET; start has one attempt on unknown outcome.
• (e) Holds: startRevylSession calls recordRent immediately after start_device returns, before readiness polling.
• (f) Holds: ready allocation reads entry.session and returns synchronously before any await.
• (g) Holds: shutdown gathers outcomes from this runtime instance's own lease map.
• (h) Not independently verified here: eager-closure guard was not run. PR body reports 882 passing.
• Finding [P1]: packages/provider-revyl/src/runtime.ts:149 — hasLiveSession/getInteractor still expose the session once shutdown begins and while release is stopping or stop-failed. These read paths use sessionForDevice without either guard, so callers can continue binding/dispatching against a device being disposed. Refuse access when shuttingDown or the lease owner is releasing, and add stopping/stop-failed coverage.
• Tests: provider-revyl focused unit suite passed once (147). New lifecycle assertions fail on base: base returns allocation after concurrent release, while head rejects it as provider-lease-releasing. Unit evidence supports the local ownership races but not live provider behavior; the live run got HTTP 401 and created zero sessions, so live device behavior remains unverified.

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	5.69 MB	5.69 MB	0 B
Package (unpacked)	5.69 MB	5.69 MB	0 B
Package (download)	1.73 MB	1.73 MB	-3 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	35.6 ms	33.4 ms	-2.2 ms
CLI --help	73.7 ms	70.4 ms	-3.3 ms

[thymikee]

Independent review at head 1f1ae76b279fff1db30e0554958ebc8a30c941cc; merge-base a0e7c7dc239922bf93adaceff20a2f7e78579146.

Verdict: no actionable code regressions found. The owner-local RevylLeaseState union has one transition owner (RevylLeaseOwner), and a lease in stopping or stop-failed cannot supply a ready device.

Checklist, with concrete traces (paths below are under packages/provider-revyl/src/):

• (a) Holds. runtime.ts:200-242 validates each caller synchronously, checks request compatibility, and publishes the owner without awaiting. lease-owner.ts:60-76 defers creation to a microtask, which runs after publication. Thus A's invalid profile fails before an owner exists; valid B starts independently. If A is already starting, B's incompatible profile fails in B's call without poisoning A's shared creation.
• (b) Holds. Release or shutdown calls end(), which synchronously replaces starting/ready with stopping and shares one disposal promise (lease-owner.ts:117-145). Disposal waits for creation before stopping the returned identity. If creation exceeds 120 seconds, it reports uncertainty, retains the owner, and installs a late continuation that re-enters end(). Concurrent release/recovery/shutdown and the failed-start continuation join the same stopping owner. lease-owner.test.ts:98-138 covers both release and shutdown crossing that budget, a late successful create, and one stop; runtime.test.ts:273-288,397-427 covers shared disposal and stop-after-create ordering. Exactly-once here means one shared logical stop attempt; failed confirmed attempts can be retried, and the HTTP client can retry the idempotent stop.
• (c) Holds for owner disposal. Creation and stop are wrapped by independent 120-second and 30-second waits (lease-owner.ts:138-177,216-228), including a transport promise that ignores abort. An unanswered stop enters stop-failed with cleanup: 'uncertain' and its pending stop handle. A retry joins that handle; late confirmation closes/removes the owner, while late failure leaves it available for a new attempt. A settled refused stop also retains the owner. Release propagates uncertainty instead of reporting success (runtime.ts:289-296). The separate restart-recovery path uses the existing HTTP client's finite per-attempt timeout/retry count (revyl-client.ts:106-119,228-239).
• (d) Holds. startDevice is a POST without retry opt-in (revyl-client.ts:152-159); send therefore permits exactly one attempt (106-110). This is the provider's own fetch client, with no SDK retry layer. Unknown create outcomes carry the deterministic session identity into cleanup rather than issuing another create. Stop retries are explicitly idempotent and do not retry creation.
• (e) Holds. Immediately after the create result returns, revyl-session.ts:123-127 parses the remote run/session identity and calls recordRent, before identity checks or any readiness await. The owner records it in starting/stopping, or updates the uncertain identity in stop-failed (lease-owner.ts:106-110).
• (f) Holds. Existing-ready allocation follows runtime.ts:207-230 without an await. A starting allocation does await creation, then re-reads entry.session before answering; if release overtook that wait, the getter supplies no session and allocation fails with provider-lease-releasing.
• (g) Holds by source trace. Each runtime constructs its own leases map and shutdown flag (runtime.ts:96-100); shutdown aggregates failures only from that map (167-177). Plugin cleanup invokes and captures each registration's shutdown separately (packages/plugins/src/registry.ts:182-191,325-342). No module-global warning accumulator or cross-instance lease state was introduced.
• (h) Holds. I compared the production eager-import walker at merge-base and head for all 437 measured entries: zero changed closure module sets. The new runtime imports remain behind the existing function-scoped plugin setup seam.

hasLiveSession and getInteractor both go through sessionForDevice (runtime.ts:149-157,312-316). The owner getter exposes a session only in ready (lease-owner.ts:98-103). Therefore both refuse during stopping and stop-failed; inventory also returns null. The new test exercises immediate retirement and post-failure refusal.

Validation: install/build passed; the single permitted provider test invocation, pnpm exec vitest run --project unit-core packages/provider-revyl, passed all 147 tests in 11 files. Reviewed source remains clean. I did not run further tests or wait for CI.

Base sensitivity: the PR reports five executed base-red regressions. I independently checked their failure traces against the base sources, but did not execute a second suite under the one-run limit: the immediate-retirement test fails because base still exposes the session; release before the first allocation await misses base's unpublished entry and allocation succeeds; both late-create budget cases fail because base waits indefinitely for start; the unanswered-stop case fails because base has no outer disposal budget. These are source-checked counterfactuals, not an independently reproduced base-red run. The bot's proposed extra stop assertion in the immediate-publication test is optional strengthening: other tests already assert the stop count and stop-after-create ordering, while that test specifically detects admission before the first await.

The unit evidence is sufficient to support this scoped in-memory ownership/lifecycle fix. It is not sufficient to claim live Revyl allocation, device commands, or remote release were verified. The reported HTTP 401 / zero-created-session run exercises authentication refusal only; successful provider integration and live cleanup remain unverified. CI was still pending when inspected and is not counted as passed evidence here.

[thymikee]

Addressed r4441116491 at 240b05012f6895ea90406d974c80102b94745903: asserted one stop after early-release disposal. Mutation proof: pass → missing stop fails (0 !== 1) → restored pass. Affected gate passed (run-39JqnV, exactCommit=true, checkoutChanged=false; 16,970 unit tests passed, one skipped). Thread replied to and resolved; new-head CI is pending.

[apex-sentinel-staging]

Apex Sentinel

Verdict: approve

Since the last review: 4 new · 0 still open · 5 no longer raised

4 finding(s): 4 nit. Verdict: approve. reached max rounds (1).

🔵 Nit (4)

Where	Finding
packages/provider-revyl/src/lease-owner.ts:19	🆕 CREATE_ANSWER_BUDGET_MS duplicates the start_device transport timeout instead of tying to it
packages/provider-revyl/src/revyl-client.ts:26	🆕 Redundant reduce generic on a numeric tuple
packages/provider-revyl/src/runtime.ts:229	🆕 A late 'ready' answer discards its result, so the still-waiting allocate reports the refusal without the start's cause
packages/provider-revyl/src/runtime.ts:382	🆕 releasingLease has no doc comment, unlike the helpers around it

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: 5 new findings on 240b050. The verdict and the full list are in the summary comment.

[thymikee]

Final round at a8ebe1f45f164249fc8d4a04221000ab1dd9d180: derived the 97 s stop-disposal budget from the client retry budget, preserved typed retirement on caller give-up, and cached ready-session reuse. Two regressions proved red→green; 149 Revyl tests and the exact-head affected gate passed (run-trpcM0, exactCommit=true, checkoutChanged=false; 16,972 unit tests passed, one skipped). All five threads have replies and are resolved; remaining style nits have reasons. This completes the final bot-nit round. New-head CI is pending.

[apex-sentinel-staging]

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

[thymikee]

Thanks for the work here. I found two lifecycle defects at a8ebe1f, and the change still needs live Revyl evidence. CI is green: 21 checks pass and none fail. There are no conflicts. Before merge, the two defects need fixes and the live run needs to be attached. I read the code only. I did not run the tests or reach Revyl, and both defects are traced from code, not reproduced.

A failed start's stop is no longer bounded by the caller's patience. At base, join raced the whole start, including the owed stop, against revylCallerWait. Now join returns when creation settles, and allocate awaits entry.endFailedStart() with no race (runtime.ts:224). That disposal can wait up to 97 s. So an allocation with a deadline or a Ctrl-C can hang long after the caller gave up, which breaks the contract in the comments at runtime.ts:186-188. The owner already owns the stop, so the caller does not need to wait for it. Every caller-facing wait in allocate should race that caller's revylCallerWait. On give-up, please report the typed give-up and let the owner finish the stop. A regression with a deadline and a never-answering stop should assert that the allocation settles at the deadline.

dispose() waits a fixed 120 s for the whole creation, but creation includes up to 240 s of readiness (lease-owner.ts:144). recordRent already holds the session and run id as soon as start_device answers (revyl-session.ts:125-126). If a release lands during a slow boot, dispose() times out and throws provider-cleanup-pending with cleanup: 'uncertain'. The hint then tells the user to end a session by hand that the owner would stop itself. The stop is also delayed until readiness gives up, so the device stays billed for up to 240 s after the release. Disposal should wait only for the answer that names the rental, bounded by that request's own transport budget. That means awaiting the rent, bounded by an exported REVYL_START_DEVICE_TIMEOUT_MS plus a margin, and then stopping the recorded session at once. A regression where a release lands mid-readiness should assert one /stop and no uncertain report. The open thread on the fixed create budget (#4169 (comment)) is the same root cause.

This change rewrites allocate, release, recovery, shutdown and when the interactor is handed out, so it needs live validation. The only live attempt returned HTTP 401 'Invalid API key' with zero sessions, so no changed route ran against Revyl. Please run with a valid key at the final head and attach output for three routes. First, connect to ready, run one device command, then disconnect. The output should show release data with providerSessionId and the Revyl session list or API showing the session ended. Second, disconnect while the first allocation is still booting. The output should show the waiting allocation rejected with provider-lease-releasing, exactly one /stop in the debug log, and the session ended in Revyl. Third, stop the daemon afterwards, with no provider-cleanup-pending.

Can this PR land first without a second Revyl-only lifecycle owner beside the shared one? #4158 already moves the same bookkeeping into provider-lease-table, edits the same files, and ships the same provider-lease-releasing behavior. Issue #4102 step 3 deletes per-provider lease bookkeeping like the new RevylLeaseOwner. The simplest path is to land #4158 and port this PR's three regressions onto it: release retires interactors, inventory and allocation synchronously, a late create is disposed once, and a give-up during release keeps the typed reason. If this PR must land first as the interim fix, the changes above keep it small, and #4158 then deletes lease-owner.ts. A maintainer should decide the merge order of #4169 and #4158 before another round.

Not blocking, and fine to take or leave: RevylLeaseOwner.stop (lease-owner.ts:185) repeats stopRented in runtime.ts:307-314, so one exported stopRevylRental(client, target) could serve both and the no-entry recovery path; the fetch stub in lease-owner.test.ts ignores the request's AbortSignal, so the 30 s attempt timeout never fires and the 97 s stop test passes against a transport that cannot behave like production, and the create test hard-codes 125_000 instead of reading the budget; and the releasingLease throw at runtime.ts:229-230 carries no cause, unlike the sibling path at runtime.ts:278.

On the earlier review threads: the one on lease-owner.ts:19/runtime.ts:229 creation budget and cause still applies, as noted above (#4169 (comment) on the create budget, #4169 (comment) on the missing cause). The remaining threads do not apply and are already resolved. These are fixed at this head: #4169 (comment) (the test now asserts one stop), #4169 (comment) (a give-up during release maps to provider-lease-releasing), and #4169 (comment) (live is stored once, with no non-null assertion). These are style only with no behavior effect: #4169 (comment), #4169 (comment), #4169 (comment), and #4169 (comment).