Repository navigation
Conversation
Revyl kept its own lease bookkeeping: one start per lease, joins bounded by each caller's budget, stop-then-forget release with one shared stop, a shutdown drain, and expired-lease recovery by the lease's own id. That bookkeeping moves to `@agent-device/capture-kit/provider-lease-table`, and Revyl now supplies only start, stop, find-by-lease and its errors. The table bounds every caller by the earlier of its deadline and the provider's start timeout, and stops a ready answer that arrives after the starting caller's deadline or cancellation. The provider runtime instance fingerprint moves next to `providerRuntimeOwner` as `providerRuntimeInstance`; Limrun's digest is unchanged, Revyl's widens to the shared 32-byte length. Limrun's lease ownership stays as is until #4141 lands, so it moves onto the table in a follow-up. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Size Report
Startup median (7 runs, lower is better):
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Apex SentinelVerdict: Since the last review: 6 new · 0 still open · 10 no longer raised 6 finding(s): 6 nit. Verdict: approve. 🔵 Nit (6)
Not in the reviewers' prompt (1 file)
Reply under an inline comment with Earlier reviewsReview of
|
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime-instance.ts:12 |
🆕 Revyl runtime-owner instance silently widens from 16 to 32 bytes; anything that persisted the old owner key stops matching |
packages/provider-revyl/src/revyl-session.ts:311 |
🆕 A readiness timeout now reports the constant 240 s budget whatever wait the caller was granted |
packages/provider-revyl/src/runtime.ts:193 |
🆕 release() on a lease whose start rejected and stays unanswered now reports a clean release for a session that was never named |
packages/provision-kit/src/provider-lease-table.ts:258 |
🆕 live() hides the session during a release, where the old map still served the inventory and interactor |
packages/provision-kit/src/provider-lease-table.ts:291 |
🆕 The shared table turns a caller's own cancellation into provider-lease-releasing whenever a release lands first |
🔵 Nit (5)
| Where | Finding |
|---|---|
packages/provider-revyl/src/revyl-backend.fixtures.ts:124 |
🆕 Fixture guard rejects only an absent Authorization header, not one that is unusable |
packages/provider-revyl/src/revyl-backend.fixtures.ts:124 |
Fixture probe filter can swallow a malformed runtime request that drops both the auth header and the /api/ prefix |
packages/contracts/src/platform-runtime.ts:121 |
🆕 providerRuntimeInstance accepts a whitespace-only field name |
packages/provision-kit/src/provider-lease-table.ts:484 |
🆕 LeaseTable.join opens with `if (entry.phase === 'ready') return { status: 'ready', session: entry.session! |
website/docs/docs/revyl.md:176 |
Hard-coded 370 s in the Revyl docs duplicates REVYL_ALLOCATION_TIMEOUT_MS |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of e40a6166 · 02.11.2026 16:52 UTC
Apex Sentinel
Verdict: approve
Since the last review: 5 new · 3 still open · 6 no longer raised
8 finding(s): 2 minor, 6 nit. Verdict: approve.
🟡 Minor (2)
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime.ts:253 |
Revyl's shared-start cancellation error loses the session id the old code reported |
website/docs/docs/revyl.md:176 |
🆕 New 370 s wait ceiling is not in the user doc that owns Revyl's lease behavior |
🔵 Nit (6)
| Where | Finding |
|---|---|
packages/contracts/src/platform-runtime.ts:80 |
🆕 Runtime-instance comment omits block size and parallelism from the digest's durable contract |
packages/contracts/src/platform-runtime.ts:137 |
🆕 New shared providerRuntimeInstance runs scrypt over the raw API key on the plugin's synchronous setup path |
packages/provider-revyl/src/revyl-backend.fixtures.ts:121 |
Revyl test fixture now silently drops unauthenticated non-/api/ requests and never records them |
packages/provider-revyl/src/runtime-instance.ts:4 |
🆕 Revyl runtime-instance doc dropped the reason the key is hashed, which now lives only in the shared helper |
packages/provider-revyl/src/runtime.ts:283 |
revylLeasePlatform re-implements requireRevylLease's guard |
packages/provider-revyl/src/runtime.ts:296 |
🆕 revylCallerBudget doc comment dropped the 'names neither' case it still implements |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of 0ae8a275 · 02.11.2026 16:26 UTC
Apex Sentinel
Verdict: request_changes
Since the last review: 7 new · 2 still open · 3 no longer raised
9 finding(s): 1 major, 4 minor, 4 nit. Verdict: request_changes.
Required fixes
- A late Revyl start is killed before its rented session is known, then stopped by an id Revyl may not hold
🟠 Major (1)
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime.ts:220 |
🆕 A late Revyl start is killed before its rented session is known, then stopped by an id Revyl may not hold |
🟡 Minor (4)
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime.ts:219 |
🆕 Revyl's driver never publishes a rental through control.rented(), so a hung start stops the whole lease id for up to ~12.5 minutes |
packages/provider-revyl/src/runtime.ts:223 |
🆕 Revyl no longer forwards the caller's deadline into readiness, so a device is held for up to 370 s after its only caller gave up at 30 s |
packages/provision-kit/src/provider-lease-table.ts:296 |
A caller whose wait has already run out is refused with a start-shaped error whose hint says the start will be stopped |
packages/provision-kit/src/provider-lease-table.ts:640 |
🆕 A rental published after a teardown finished is stopped outside the table, and a failure there is never reported |
🔵 Nit (4)
| Where | Finding |
|---|---|
packages/provider-revyl/src/revyl-backend.fixtures.ts:121 |
🆕 The fixture's new auth/path gate silently drops unauthenticated non-/api requests instead of recording them |
packages/provider-revyl/src/revyl-backend.fixtures.ts:124 |
🆕 Fixture request filter checks for any authorization header and any /api/ path, not the fixture's key and Revyl's prefix |
packages/provider-revyl/src/runtime-instance.ts:9 |
🆕 Revyl's runtime instance changes value (16-byte to 32-byte digest), which also changes every runtimeOwnerKey a Revyl owner produces |
packages/provider-revyl/src/runtime.ts:298 |
gaveUp driver callback can throw instead of reporting |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of c85ab811 · 02.11.2026 15:58 UTC
Apex Sentinel
Verdict: approve
Since the last review: 4 new · 1 still open · 5 no longer raised
5 finding(s): 2 minor, 3 nit. Verdict: approve.
🟡 Minor (2)
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime.ts:220 |
🆕 Revyl allocation timeouts are now silently truncated to 370 s, ignoring the caller's LeaseLifecycleContext.deadline |
packages/provision-kit/src/provider-lease-table.ts:608 |
🆕 Shutdown never abandons a stalled start: the drain waits the full start timeout (370 s for Revyl) and the provider keeps renting |
🔵 Nit (3)
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime.ts:298 |
🆕 New revylLeasePlatform error carries no reason, unlike every other Revyl lease failure |
packages/provision-kit/src/provider-lease-table.ts:194 |
🆕 LeaseEntry.start is seeded with a never-valid failed outcome |
packages/provision-kit/src/provider-lease-table.ts:411 |
A shared start's failure names the first caller's budget and cancellation to every other waiter |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of be5dab00 · 02.11.2026 15:19 UTC
Apex Sentinel
Verdict: approve
Since the last review: 3 new · 3 still open · 5 no longer raised
6 finding(s): 2 minor, 4 nit. Verdict: approve.
🟡 Minor (2)
| Where | Finding |
|---|---|
packages/provider-revyl/src/revyl-backend.fixtures.ts:124 |
Fixture's new missing-Authorization guard rejects before the request is recorded, so an unauthorized request the runtime really sent becomes invisible |
packages/provider-revyl/src/runtime.ts:223 |
Revyl start no longer receives the caller's deadline, so readiness always spends its full 240 s |
🔵 Nit (4)
| Where | Finding |
|---|---|
packages/contracts/src/platform-runtime.ts:85 |
🆕 scrypt's r and p are inlined next to the constants that carry the same durability promise |
packages/provision-kit/src/provider-lease-table.ts:95 |
ProviderLeaseDriver.stop has no deadline, so stopTimeoutMs only stops the table from waiting |
packages/provision-kit/src/provider-lease-table.ts:525 |
🆕 stopRentals re-judges an already-tried rental through rental.stopping, an implicit invariant on the stop bookkeeping |
packages/provision-kit/src/provider-lease-table.ts:530 |
🆕 The shared table invents three user-visible error messages the provider is meant to own |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of 03fc49e1 · 02.11.2026 13:44 UTC
Apex Sentinel
Verdict: approve
Since the last review: 7 new · 1 still open · 2 no longer raised
8 finding(s): 4 minor, 4 nit. Verdict: approve.
🟡 Minor (4)
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime-instance.ts:12 |
🆕 Revyl's runtime-instance digest changes, re-identifying every Revyl owner across a daemon upgrade |
packages/provision-kit/src/provider-lease-table.ts:300 |
🆕 LeaseTable.recover is the only entry point that does not check this.shuttingDown (allocate throws `driver.shutt… |
packages/provision-kit/src/provider-lease-table.ts:498 |
🆕 A teardown reports only the first unconfirmed rental, so a second billed rental of the same lease is neither stopped nor reported by shutdown |
packages/provision-kit/src/provider-lease-table.ts:548 |
🆕 LeaseEntry.answer is a promise resolved only by markAnswered, and untilAnswered races it against a timer, so a … |
🔵 Nit (4)
| Where | Finding |
|---|---|
packages/provider-revyl/src/revyl-backend.fixtures.ts:124 |
Revyl fixture backend now drops every request without an authorization header, so a test asserting a recorded request must always send the key |
packages/provider-revyl/src/revyl-session.ts:18 |
🆕 Revyl allocations with no caller deadline now fail at 370 s, and that cap is derived from constants in two files with nothing enforcing the ordering it depends on |
packages/contracts/src/platform-runtime.ts:118 |
🆕 The shared providerRuntimeInstance silently drops '' principal values, which the old per-provider JSON digest carried |
packages/provider-revyl/src/runtime.ts:218 |
🆕 Revyl's driver ignores control.deadline, so readiness keeps polling Revyl for up to 240 s after the caller that asked for a shorter budget was refused |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of f82368bd · 02.11.2026 13:04 UTC
Apex Sentinel
Verdict: approve
Since the last review: 2 new · 1 still open · 7 no longer raised
3 finding(s): 1 minor, 2 nit. Verdict: approve.
🟡 Minor (1)
| Where | Finding |
|---|---|
packages/provider-revyl/src/revyl-backend.fixtures.ts:124 |
Fixture backend now answers 404 with no record for any request lacking an Authorization header, hiding a client that omits the header |
🔵 Nit (2)
| Where | Finding |
|---|---|
packages/provider-revyl/src/revyl-session.ts:18 |
🆕 REVYL_ALLOCATION_TIMEOUT_MS hard-codes its 10-second margin |
packages/provider-revyl/src/runtime.ts:285 |
🆕 revylLeasePlatform can throw from inside the driver's error-reporting callbacks |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of edb5034b · 02.11.2026 12:43 UTC
Apex Sentinel
Verdict: approve
Since the last review: 6 new · 2 still open · 5 no longer raised
8 finding(s): 3 minor, 5 nit. Verdict: approve.
🟡 Minor (3)
| Where | Finding |
|---|---|
packages/contracts/src/platform-runtime.ts:113 |
🆕 ProviderRuntimePrincipalField allows a repeated name whose value is undefined |
packages/provider-revyl/src/revyl-backend.fixtures.ts:124 |
Fixture now silently 404s any request without an Authorization header, so an unauthenticated Revyl call is invisible to every path assertion |
packages/provider-revyl/src/revyl-session.ts:20 |
🆕 Revyl's stop budget is now unbounded: the RevylClient request timeout is absent from REVYL_ALLOCATION_TIMEOUT_MS |
🔵 Nit (5)
| Where | Finding |
|---|---|
packages/provision-kit/src/provider-lease-table.ts:260 |
🆕 A caller whose budget is already spent still joins an existing start and keeps it wanted for a tick |
packages/contracts/src/platform-runtime.ts:105 |
🆕 providerRuntimeInstance moved a blocking scrypt (N=16384) into @agent-device/contracts |
packages/provider-revyl/src/runtime.ts:273 |
🆕 The table's start cleanup scope drops the lease id that its own type carries |
packages/provider-revyl/src/runtime.ts:300 |
Stale comment: the table shortens a caller's deadline, it does not cut it to REVYL_ALLOCATION_TIMEOUT_MS |
packages/provision-kit/src/provider-lease-table.ts:435 |
🆕 A caller's long-lived abort signal retains the lease entry after its allocation ends |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of c7a089de · 02.11.2026 11:25 UTC
Apex Sentinel
Verdict: request_changes
Since the last review: 3 new · 4 still open · 11 no longer raised
7 finding(s): 1 major, 1 minor, 5 nit. Verdict: request_changes.
Required fixes
- A failed Revyl start now waits the full 370 s allocation budget before its caller is answered
🟠 Major (1)
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime.ts:220 |
A failed Revyl start now waits the full 370 s allocation budget before its caller is answered |
🟡 Minor (1)
| Where | Finding |
|---|---|
packages/provider-revyl/src/revyl-backend.fixtures.ts:121 |
Fixture now hides every GET / from all revyl test assertions, not only external probes |
🔵 Nit (5)
| Where | Finding |
|---|---|
packages/contracts/src/platform-runtime.ts:80 |
🆕 providerRuntimeInstance silently changes Revyl's runtime-instance digest format |
packages/contracts/src/platform-runtime.ts:99 |
The provider-normalization invariant shared with providerRuntimeOwner is kept by copying it |
packages/contracts/src/platform-runtime.ts:99 |
🆕 providerRuntimeInstance normalizes only provider, while every other principal field is order- and case-sensitive durable material |
packages/provision-kit/src/provider-lease-table.ts:256 |
releasing() runs outside the try/catch in allocate, so a throwing driver aborts the teardown that just failed to stop a billed device |
packages/provision-kit/src/provider-lease-table.ts:485 |
🆕 Shared lease table keeps a releasing entry for every lease forever when a stop and findByLease both fail permanently |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
Review of b25dc6c5 · time not recorded
Apex Sentinel
Verdict: approve
Since the last review: 6 new · 1 still open · 10 no longer raised
7 finding(s): 2 minor, 5 nit. Verdict: approve. reached max rounds (1).
🟡 Minor (2)
| Where | Finding |
|---|---|
packages/provider-revyl/src/runtime.ts:219 |
🆕 Revyl never publishes control.rented(), so a stalled start_device holds release, recovery and shutdown for the full 370 s allocation timeout |
packages/provider-revyl/src/runtime.ts:223 |
🆕 Revyl's driver forwards the starting caller's abort into start_device, so the table's "a joiner still waiting keeps the device" rule does not hold for Revyl |
🔵 Nit (5)
| Where | Finding |
|---|---|
packages/contracts/src/platform-runtime.ts:80 |
providerRuntimeInstance's key-length comment cites Limrun's 'runtimeInstance', which is an explicit-instance option, not a digest |
packages/contracts/src/platform-runtime.ts:99 |
🆕 providerRuntimeInstance takes an untyped principal whose digest depends on field order, so only comments protect durable descriptors |
packages/provider-revyl/src/revyl-backend.fixtures.ts:124 |
🆕 Fixture backend silently 404s and stops recording every request outside /api/, hiding future request-path regressions from all Revyl request assertions |
packages/provider-revyl/src/runtime.ts:284 |
🆕 revylLeasePlatform throws a bare TypeError inside driver error factories |
packages/provision-kit/src/provider-lease-table.ts:392 |
🆕 A rejected findByLease is reported as cleanup-pending with a cause the provider never saw |
Not in the reviewers' prompt (1 file)
pnpm-lock.yaml: generated or a lockfile, so its lines were left out
![apex-sentinel-staging[bot]](https://avatars.githubusercontent.com/in/5196457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Apex Sentinel: 7 new findings on d4151ce. The verdict and the full list are in the summary comment.
Renting a cloud device prepares it for execution, which ADR 0019 gives to provision-kit; capture-kit owns capture, snapshot and recording. The table, its tests and its export move unchanged, and Revyl depends on provision-kit instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of the shared lease table found routes that could forget or hand out a billed device: - Release, recovery, a failed start and shutdown now end a lease through one teardown that they all join. It fences the lease in the same step, so an allocation that arrives or is still waiting is refused with `releasing` instead of handed a device that is being stopped. - A recovery of a lease this process holds nothing for publishes an entry before its lookup, so concurrent routes share one lookup and one stop, and a failed stop keeps the handle it found. - A start that rejects and that find-by-lease cannot name stays owed: the lease is refused and kept pending until a teardown finds it. - A start can publish its rental before it answers, and a teardown waits for an answer for at most the start timeout before it stops that rental. An optional stop timeout bounds each wait for a stop, and the next teardown joins the stop still in flight. - A ready device goes to any caller still waiting; it is stopped only when every caller gave up. `gaveUp` reports the caller's own budget. - Joinability is an `allocate` option instead of a `requestOf` read, and join clears its timer once the caller is answered. Both providers pin their runtime-instance digest through their own wrapper, and ADR 0007 and ADR 0019 describe the table as built. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 15 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
Apex Sentinel: 11 new findings on e429ee1. The verdict and the full list are in the summary comment.
…oject The lease table moved to provision-kit, and the layering suite requires each package's project references to match its manifest dependencies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…teps Fallow's cognitive-complexity bar flagged runStart and runTeardown. Each step now has its own method (call the start, record its answer, name a failure, end a failed start; name a rental, finish a teardown), with no change in behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Apex Sentinel: 7 new findings on 85259a8. The verdict and the full list are in the summary comment.
Second review round on the shared lease table: - A ready answer goes to a caller only while that caller's own signal and deadline still hold, so a caller whose deadline passed before its timer ran is not handed a device; nobody waiting means it is stopped. - The session a start rented is recorded even when nobody takes it, so a release that ended the start reports the device (Revyl: `revylRunId`). - A caller whose budget is already spent sends no start. - Shutdown repeats its drain until it has ended every entry, including one a recovery or release opened while it ran. - Revyl carries its platform in the lease request, names its cap `REVYL_ALLOCATION_TIMEOUT_MS` with a margin over start plus readiness, and builds its driver with `createRevylLeaseDriver`. - ADR 0007 records the inventory and interactor effect of the fence, the `provider-lease-releasing` reason and Revyl's in-process digest change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
The lease table refactor looks correct in the diff at f504227, but the Revyl route still needs a live run before merge. No conflicts, and all checks are green. The smoke lanes use local simulators and emulators, so they do not cover Revyl. Revyl's lease lifecycle now runs through a new owner with changed live behavior (see runtime.ts). Overtaken allocations are refused, an unwanted ready device is stopped at its answer, and waits are capped at 370 s. The validation is unit and fixture only, while the Limrun precedent in #4141 had a live release run. If the fixture differs from real Revyl (for example /stop timing right after start_device, or a 404-as-gone answer), a billed device could keep running or a release could be reported that Revyl never confirmed. Please run once against real Revyl at f504227: connect a lease, run one command on the rented device (a snapshot), then disconnect. Record the providerSessionId, the single POST .../sessions//stop that was confirmed, and Revyl showing no running session afterwards. Also stop the daemon while a lease is held, and show that shutdown stops the device. Please add a check:affected receipt at exactly f504227; the PR's numbers are from 85259a8. Not blocking, and you can take or leave these: The P2 inline thread on shutdown versus provider-lease-releasing (thread) still stands, as do the three lower-priority inline threads. I did not run the test suites or check:affected, and I could not run against real Revyl, so the fixture's /stop and 404 answers are unverified against production. |
…warning ADR 0007 names `provider-lease-releasing` for a lease that a release, recovery or failed start is ending and `runtime-shutting-down` after shutdown began, and says shutdown re-drains leases a recovery opens. The digest comment covers only derived instances and the inputs that change them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Apex Sentinel: 9 new findings on 9df80ce. The verdict and the full list are in the summary comment.
The fixture listens on an ephemeral localhost port that any process on the host can reach, and a stray `GET /` from one made the exact-path assertion in the lease-start test fail under host load. Requests outside `/api/` are answered 404 and not recorded as the runtime's calls. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Apex Sentinel: 6 new findings on b25dc6c. The verdict and the full list are in the summary comment.
The Revyl driver passed the starting caller's signal into the start, so that caller's cancellation ended readiness for every caller that joined it. The start now gets only the caller's deadline; the table stops the device once it is ready if no caller still wants it. A lease without a Revyl platform is refused with a typed error, the fixture ignores only a bare `GET /` probe, and the table documents which waits it bounds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
|
I reviewed c7a089d. Most of the earlier review (#4158 (comment)) is addressed, but this commit adds one regression, and the live Revyl evidence is still missing. A cancelled Revyl connect now keeps a billed device provisioning for up to about 4 minutes. c7a089d stops forwarding the starting caller's signal into The live Revyl run is still needed, now including the cancel path. At the final head, against real Revyl: (1) connect a lease, take one snapshot, disconnect, and record the providerSessionId, the single confirmed On the threads: the cubic P2 thread on the fixture's stray traffic (#4158 (comment)) does not apply, because the fixture listens on 127.0.0.1:0, every runtime path is an absolute /api/v1 path, and an /api/ filter would hide a runtime request that went wrong. The two newer threads on a bare GET / and on findByLease rejecting (#4158 (comment), #4158 (comment)) do not apply either. The earlier shutdown-versus-releasing P2 thread and the 370 s and fingerprint wording threads are fixed at this head. Smoke Tests is still in progress and does not exercise Revyl. I did not run the tests or real Revyl; the 4-minute window comes from reading the readiness and table code. |
Every runtime request carries the key, so a request without one is another host process probing the fixture's ephemeral port. It is answered 404 and not recorded; any runtime request is recorded at any path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ncipal Coordinator follow-up on the Apex Sentinel findings at d4151ce: - The start no longer sees any single caller's signal or deadline. It runs under the provider's start timeout, and the table aborts the start's own signal only when no caller still waits. A first caller that hangs up or runs out of time ends only its own wait; a joiner still waiting gets the device. Revyl passes that signal to its start. - `providerRuntimeInstance(provider, principal)` takes an ordered list of named facts. An unset fact is the same account as an absent one; an empty or repeated name, or a principal with no value, is refused. The serialization is unchanged, so Limrun's and Revyl's pinned digests stay byte-identical. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/contracts/src/platform-runtime.ts">
<violation number="1" location="packages/contracts/src/platform-runtime.ts:113">
P2: A `__proto__` principal field is silently dropped because `facts` is a normal object, allowing distinct principals to share an instance or causing a valid sole fact to be rejected. Use a null-prototype facts object or reject prototype names.</violation>
</file>
|
Thanks for the update. I reviewed 8393630 against the earlier review. The driver now forwards the abort signal, and the fence on late callers is in place, so those parts are fixed. One defect and two validation gaps remain, and the code is not ready to merge yet. A release still leaves an unanswered Revyl start running. The Revyl-route regression from the earlier review is still missing (runtime.test.ts:739). The file is unchanged since c7a089d. If The lease lifecycle is a device-facing path (runtime.ts:223), and its only validation is unit and fixture tests. All 23 checks are green, but the smoke lanes use local simulators and emulators. No CI job reaches Revyl, so green CI says nothing about this route. I did not run real Revyl, so the fixture's Could the entry's ending state collapse into one step? A private One open inline thread still applies: the rental named after an in-flight stop is left out of the owed list and the pending-cleanup handles. Revyl cannot reach it today, but the Limrun migration can. On break, please append every unstopped, untried rental with the cause "waiting for the stop of ". I also did not trace the daemon's close order. If the request drain aborts an in-flight connect's signal before plugin shutdown, a daemon stop during readiness ends the start through abandonment. Otherwise the drain waits for readiness, past the 5 s plugin budget. The other inline threads are fixed at 8393630 or benign, and they are already resolved: r4442468220, r4442468238, r4442468250, r4442515123, r4442515136, r4442515147, r4442643269, r4442643285, r4442710139, r4442992845, r4443109893, r4443110368, r4443111272, r4443111286. Before merge, please fix the release-during-readiness defect, add the two Revyl-route regressions, post the three live Revyl runs with an exact-head |
…light When a stop was still in flight, the rentals named after it were left untried, as intended, but also left out of the cleanup report. The pending report now names every rental still rented, with the ones waiting behind the in-flight stop given that as their cause. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Apex Sentinel: 2 new findings on be5dab0. The verdict and the full list are in the summary comment.
…henticated probe is - scrypt's block size and parallelism are named beside its cost and key length, under one comment saying all four feed the durable digest. - `RentalRecord.stopping` documents that it is the fence that keeps a later rental's stop from overtaking an in-flight one. - The Revyl fixture ignores only a request with neither the API key nor a path under Revyl's API, so a runtime call that drops the key is still recorded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
I reviewed be5dab0 against the earlier review (#4158 (comment)). One defect from that review is still open, and the Revyl tests and live validation are still missing. All 24 checks pass, but the smoke lanes use local simulators and emulators and no CI job reaches Revyl, so green CI says nothing about the changed device path. The defect is unchanged: teardown() sets Both Revyl-route tests asked for earlier are still missing; the delta since a42bc35 touches no provider-revyl file. The table tests use a fake driver, so deleting This PR rewrites the Revyl lease lifecycle (start abandonment, the fence, the stop at the answer, the 370 s cap) and it has only unit and fixture validation. The check:affected receipt is for 8393630, and be5dab0 changed production lines after it. If real Revyl differs from the fixture's /stop timing or its 404-as-gone answers, a billed device can keep running or a release can be reported as confirmed when Revyl never confirmed it. At the final head, after the fix above, please run against real Revyl and post: (1) connect, one snapshot, disconnect, with the providerSessionId, the single confirmed POST /api/v1/execution/device/sessions//stop, and no running session listed afterwards; (2) a held ready lease with the daemon stopped, showing the shutdown drain sent the confirmed /stop and the daemon reported no pending cleanup; (3) a cancel while health is still 503, with the cancel and /stop timestamps, the stop within about one 2 s poll. Also post a check:affected --run receipt with exactCommit=true for that same head. Not blocking: the synthesized cause for a rental that waits does not name the rental whose stop holds it back, and Revyl's cleanupPending forwards it as the failure text at runtime.ts:265, so naming the earlier rental's key there would help; take or leave it. On the open threads: the cubic-dev-ai P2 at #4158 (comment) is fixed at this head (every unstopped rental is now reported, covered by the test at provider-lease-table.test.ts:523), so it can be resolved. The scrypt naming nit (#4158 (comment)) is addressed in the newer commit c85ab81. The ordering nit (#4158 (comment)) still applies and does not block. I did not run the table or Revyl tests or real Revyl, so the 240 s window comes from reading the readiness budget. I also did not trace whether the daemon's close order aborts an in-flight connect before plugin shutdown; that was left open earlier too. Before merge, the lease-table fix, the two Revyl tests and the live runs with an exact-head receipt need to land. Is #4169 meant to be a second lease owner for Revyl alongside this table? I only read its file list and state, so a maintainer decision on which one owns Revyl leases is needed. This review covers be5dab0. The newer head c85ab81 only names the scrypt parameters, adjusts a fixture and expands a comment, so the defect, the two Revyl tests and the live runs above still apply. |
There was a problem hiding this comment.
Apex Sentinel: 1 new finding on c85ab81. The verdict and the full list are in the summary comment.
Each lease entry now has a generation and one phase, `starting`, `ready` or `ending`, replacing the flags that encoded it before. Only `starting` → `ready` and any phase → `ending` happen, each in one synchronous step. After every await, the table checks that the entry is still its lease's current generation and in the expected phase before it hands out a session or forgets the lease, so a stale start answer only ever stops its device. Shutdown is a synchronous fence: an allocation, or a recovery of a lease with no entry, is refused with `shuttingDown` once it begins, so the drain ends every entry in one pass. ADR 0007 states the fence, and the Revyl troubleshooting guide names `provider-lease-releasing`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
400 seeded runs issue allocations, aborts, releases, recoveries, one shutdown and time passing in bursts, and answer every start and stop in a seeded order. Each run checks that no session is handed out after its lease ended or its device stopped, that no device the table took on is left running or unreported when shutdown settles, that at most one stop of a device is in flight, and that a failed stop is retried until the final drain confirms it. A failing seed names itself for replay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
R84 colocation reads a test named <module>-<aspect>.test.ts as its module's; a dotted aspect names no module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Apex Sentinel: 1 new finding on 0ae8a27. The verdict and the full list are in the summary comment.
…s lease The seeded sweep, extended to starts that publish their device early, found a teardown that awaited its stops, found none owed, and then forgot the lease even though the start had recorded a rental meanwhile: shutdown resolved with that device still rented (seed 92). The teardown now re-checks synchronously for an unnamed or unstopped rental before it forgets the lease, and loops if one appeared. The sweep now publishes some devices before their start answers, does not rent a published device twice, and accepts a shutdown report that names a still-unnamed rental by its lease. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`connect` saves the profile and rents nothing; `open` allocates the new lease that rents a fresh device. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
There was a problem hiding this comment.
Apex Sentinel: 3 new findings on e40a616. The verdict and the full list are in the summary comment.
The `provider-lease-releasing` hint now names `open`, as the guide does, since `connect` rents nothing. The guide states that an `open` with no deadline stops waiting after 370 s, and Revyl's instance wrapper keeps the reason its principal is digested. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The earlier findings at 8393630 are mostly fixed at e40a616, but one defect and two gaps in validation remain. CI is green with all 24 checks passing, but no CI job reaches the Revyl route this PR changes, so green CI does not cover runtime.ts or the table's Revyl driver. There are no conflicts. The release-during-start defect is unchanged: teardown() sets the phase to 'ending' but never aborts entry.abandon, and stillWanted (:741-749) still counts the waiters of an ending entry, so abandonIfUnwanted (:511) does not fire. When a canceled joiner makes the daemon release the lease, or a client calls leases.release during an The two Revyl regressions asked for earlier are still missing; only the fixture changed in provider-revyl tests. The release-while-start-runs test asserts no timing, so it passes with or without the abort, and the cancel and deadline tests (:706, :739) still pass if This PR rewrites the Revyl lease lifecycle, which is a device-facing path, and its only validation is unit and fixture tests. The start, fence, stop and 370 s cap have no live run, and the check:affected for e40a616 shows a failing unit stage with no passing exact-head receipt. If real Revyl differs from the fixture's /stop timing or its 404-as-gone answer, a billed device can stay running. After the fix above, please run against real Revyl at the final head and post: (1) connect, one snapshot, disconnect, with the providerSessionId, the single confirmed POST /api/v1/execution/device/sessions//stop, and no running session listed afterward; (2) hold a ready lease and stop the daemon, showing the drain sent the confirmed /stop and reported no pending cleanup; (3) cancel an open while /health is still 503, with the cancel and /stop timestamps, the stop within about one 2 s poll. Please also post a passing Could the ending state fold into one private end(entry) in provider-lease-table.ts? It would be the only writer of 'ending', abort entry.abandon when the start has not answered, and open or join the one teardown. runTeardown would then decide pending or forget from entry.rentals synchronously after every await and clear entry.ending in that same tick, not in .finally. A rental published by control.rented would then either land before the decision and be seen, or land after 'ending' clears and start a fresh teardown. That would remove followUpQueued and the :544 re-check, stillWanted would need no phase check, and abandonIfUnwanted would reduce to "the last waiter left, so end(entry)". On the open inline threads: the cubic-dev-ai thread on the stale reconnect hint (#4158 (comment)) is addressed by the newer commit 327a216, which points the hint at I did not run any test or check:affected, so the claim that the e40a616 unit failures are in daemon-client tests outside this diff is unverified. I did not run the interleaving sweep with provider-lease-table.ts:544 removed, so seed 92 as the regression for 8db5cc9 is unverified. I could not run real Revyl, so the fixture's /stop timing and its 404-as-gone answer are unchecked. The daemon release-on-joiner-cancel route comes from the earlier trace and the comment at runtime.test.ts:218-220, not a new trace. The ordering concern behind the end(entry) question comes from reading the microtask order around Before merge, the unanswered start must be aborted on teardown, the two Revyl regression tests added, and the three live Revyl runs and an exact-head check:affected receipt posted. This review covers e40a616. The newer head 327a216 only changes the reconnect hint and docs wording, so the lease-table defect, the two Revyl tests and the live runs above still apply. |
…xists Shutdown's drain needs one pass because nothing registers an entry once it begins: allocation and entryless recovery are refused, and a release only joins an existing entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Apex Sentinel: 1 new finding on 327a216. The verdict and the full list are in the summary comment.
|
#4169 is closed in favor of this PR, so that Revyl has one lease owner. Please port its three Revyl regressions here. They are the Revyl tests the review asks for. Each test is linked at the final #4169 head:
The #4169 review found two defects. Your table design must not repeat them:
Live Revyl run: the main checkout's |
…ness A release, recovery or shutdown that took over a lease mid-start waited for the start to finish readiness before it could stop the device, up to Revyl's whole allocation timeout. The table now aborts the start's own signal as soon as a teardown takes the lease, since no caller can receive that device any more, and reports the lease's end, not the aborted start, to callers still waiting. Revyl publishes its session through `control.rented` the moment `start_device` answers with it, the first moment Revyl holds it, and a release of a session that never became ready still reports its run id. New test: a release during a stalled readiness stops the device at once. The two table tests the seeded sweep fully covers are folded into it to keep the change within its budget, and Revyl's lease tests share one refusal helper to stay within the file's line cap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Apex Sentinel: 4 new findings on b6aa27b. The verdict and the full list are in the summary comment.
|
At b6aa27b, teardown now aborts readiness and the earlier cleanup reporting and retry guidance issues are fixed, but the give-up path still loses the typed teardown reason. Smoke Tests is still in progress with no failure excerpt; its local device routes do not exercise these provider lease changes, and no CI job reaches live Revyl. There are no conflicts. Before merge, fix the ending-entry refusal and add its regression test, then provide the three live Revyl runs and a passing check:affected --run receipt for the final head. In join’s give-up path, a caller that aborts or reaches its deadline after release starts receives request_canceled or provider-not-ready instead of provider-lease-releasing; shutdown similarly loses runtime-shutting-down. This can send a client down the wrong recovery path, including retrying a lease that is still releasing. Every refusal of an ending or replaced entry must name the ending, with shutdown taking precedence, whichever path answers the caller. Route giveUp, deliver and startFailure through one private refusal helper. Port #4169’s give-up-during-release regression to Revyl: delay start_device, release, then abort the caller, and assert provider-lease-releasing and exactly one /stop. Confirm that it fails on this commit. The Revyl lifecycle changes still lack live evidence, and the check:affected receipt covers 831f01b, before the final production edits. Unit and fixture evidence cannot confirm real stop timing, 404 handling or cleanup after aborted readiness, so a billed session could remain running or release could appear confirmed without provider confirmation. At the final head, export REVYL_PARTNER_API_KEY as REVYL_API_KEY and provide three real Revyl runs: open, take one snapshot and close, showing providerSessionId, one confirmed POST /api/v1/execution/device/sessions//stop and no running session afterward; stop the daemon with a ready lease held, showing the drain’s confirmed /stop and no provider-cleanup-pending; and release or cancel while /health still returns 503, showing release or cancel and /stop timestamps within about one 2 s poll and no running session afterward. Provide a passing check:affected --run receipt with exactCommit=true for that same head. I did not run the table or Revyl tests, mutations or real Revyl; the release-test coverage assessment comes from tracing the fixture’s delayed health response, and I did not trace daemon close order. Not blocking: take or leave a sole-caller cancellation test during a roughly 2 s health delay that proves request_canceled and one /stop before health answers, a runtime test that proves interactors and inventory disappear and repeat allocation stays refused during and after a failed release, an assertion that overlapping teardown never searches for a held lease, and an updated abandon description with teardown as its sole abort site. No open inline threads still apply. These threads can be resolved: deadline handover correctly refuses an over-budget caller; allocation options is a style preference; cleanup scope already carries leaseId for start; rental publication is already scoped to before the answer; deadline clocks agree at the deadline; unstopped rentals are now all reported; scrypt parameters are named; stop fence is documented; placeholder start cannot be observed before assignment; retry documentation now says to open again; early publication coverage now exists; found-handle cleanup has table coverage; request authentication is asserted by awaited requests; releasing hint now matches the docs; 370 s documentation gives the user the required number; instance derivation is already documented; key hashing has a shared explanation; unnamed-session cleanup now returns pending and searches for the rental; field names are code constants; session assertions follow assignment before the ready phase. |
…borts Teardown is now the one place that aborts a start's signal: when a release, recovery, shutdown, failed start or the last waiter leaving ends a lease whose start has not answered. `stillWanted` is false for an ending entry, so nothing can count a fenced lease's waiters, and the last-waiter path only ends the entry. Two Revyl regressions pin that the abort reaches readiness: a sole caller that hangs up during readiness has its session stopped before the delayed health answer, and a release that lands while the first caller still waits stops the session and refuses that caller before health answers. Both fail if the teardown abort, or Revyl's `signal: control.signal`, is removed. The table test those cover is folded in to keep the change within its budget. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Evidence for the e40a616 review, at head 0c401f91. Teardown aborts an unanswered startConfirmed at
Not applied: the full private 2. Two Revyl regressions in
|
| Removed | (a) | (b) |
|---|---|---|
The abort in teardown() (if (!entry.answered) entry.abandon.abort();) |
fails | fails |
signal: control.signal in packages/provider-revyl/src/runtime.ts |
fails | fails |
3. Live Revyl runs at 0c401f9
How they ran:
- The key was sourced from
/Users/michal/Developer/agent-device/.envby absolute path and never printed. connect revylrequires a hosted--provider-appbuild, and none is available. So, as in fix(revyl): drag duration, hierarchy 500 retry, warm-start guidance #4144, I drove the realcreateRevylRuntimefrom this head againstbackend.revyl.ai, with afetchwrapper that logs only time, method, path and status.runtime.shutdown()is the drain thatdaemon stopcalls on this runtime.- Revyl has no session-list endpoint:
GET /api/v1/execution/device/sessionsand.../activeboth return 404. The "nothing running" evidence is therefore Revyl's own per-run status (worker-connection).
(1) Connect, one snapshot, disconnect. providerSessionId bde30aba-2973-4d10-9009-a325c92b5e93, run f55ec532-c0fc-4b76-88d3-6f8a6e1fd017.
19:06:17.883Z POST /api/v1/execution/start_device -> 200
19:06:21.392Z GET .../device-proxy/f55ec532.../health -> 200 (ready)
19:06:26.559Z GET .../device-proxy/f55ec532.../hierarchy -> 200 (snapshot)
19:06:27.368Z POST /api/v1/execution/device/sessions/bde30aba-2973-4d10-9009-a325c92b5e93/stop -> 200 (the only /stop)
19:06:37.140Z worker-connection f55ec532... status stopped (steady through 19:07:16Z)
Revyl confirms /stop at once and reports the run stopped within about 10 s.
(2) Hold a ready lease, then stop the daemon. Session b323240a-cd05-4054-9ea8-1530f8dde5e6, run 52548cdc-0f93-4cad-8b13-3341ceb1d274.
19:07:45.656Z READY; shutdown (drain) begins
19:07:46.452Z POST .../sessions/b323240a-cd05-4054-9ea8-1530f8dde5e6/stop -> 200
19:07:46.452Z shutdown resolved: no pending cleanup
19:07:57.856Z worker-connection 52548cdc... status stopped
(3) Cancel an open during readiness. Session 6bad564e-7dc4-468f-bad7-1f9e91c75d97, run 6d2d91ae-64fc-4a5f-bfbc-756eb30a1b22.
Real Revyl answered readiness with 200 and device_connected=false rather than 503 in every attempt, because its pool had a warm simulator. The cancel therefore fired on the first not-yet-connected readiness answer.
19:09:23.758Z GET .../health -> 200 device_connected=false; CANCEL
19:09:23.760Z allocate refused: request_canceled; the in-flight readiness read aborted (AbortError)
19:09:24.528Z POST .../sessions/6bad564e-7dc4-468f-bad7-1f9e91c75d97/stop -> 200 (0.77 s after the cancel)
19:09:34.980Z worker-connection 6d2d91ae... status stopped
An earlier attempt, run 04b53eb5-fcc5-4bc5-b316-c2c9251990df, armed its cancel on a 503 that never came. Its allocation completed, and the final drain stopped it.
All four runs started are stopped: f55ec532, 52548cdc, 04b53eb5 and 6d2d91ae each report stopped at 19:09:48–50Z.
4. Exact-head receipt
pnpm check:affected --run --head 0c401f9d46 passed with exactCommit=true, receipt run-0wyZcF: 16,887 unit tests passed (1 skipped), 1,234 related.
The stale fixture message (cubic r4441743444) needed no change: the head's filter, added in c85ab8119a, already keeps keyless non-API host probes out of the recorded requests. The diff is 2,499 changed lines.
…tory throws When a late start answer lands on an ending lease, the table builds the waiters' error with the driver's `releasing()` before it runs the follow-up teardown that stops the late rental. A driver whose factory threw skipped that teardown and left the device rented. The failure is now built inside a guard, and the follow-up teardown always runs. Tests: a start that fails 50 ms in answers its caller at once, stops what it rented and frees the lease for a new allocation; and a driver whose `releasing()` throws still has a late answer's device stopped. The table test that the seeded sweep's stop-overlap check covers is folded in, and the table's type docs are condensed to stay within the change's budget. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/provision-kit/src/provider-lease-table.test.ts">
<violation number="1" location="packages/provision-kit/src/provider-lease-table.test.ts:417">
P2: This test can fail intermittently under CI load because it uses a real 50 ms timer and asserts a wall-clock bound of 1 second. Schedule the rejection on the next turn and rely on the `/lost answer/` assertion; a regression that waits for the 60-second start timeout will still exceed the test timeout.</violation>
</file>
<file name="packages/provision-kit/src/provider-lease-table.ts">
<violation number="1" location="packages/provision-kit/src/provider-lease-table.ts:64">
P2: The condensed `stopTimeoutMs` doc drops the note that the table does not bound `findByLease`. The table awaits `findByLease` with no timeout, so a driver author could leave it unbounded and hang release. Restore that sentence.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| const allocating = provider.table.allocate(lease(), {}); | ||
| await settle(); | ||
| const startedAt = Date.now(); | ||
| setTimeout(() => provider.failStart(0, new Error('lost answer')), 50); |
There was a problem hiding this comment.
P2: This test can fail intermittently under CI load because it uses a real 50 ms timer and asserts a wall-clock bound of 1 second. Schedule the rejection on the next turn and rely on the /lost answer/ assertion; a regression that waits for the 60-second start timeout will still exceed the test timeout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provision-kit/src/provider-lease-table.test.ts, line 417:
<comment>This test can fail intermittently under CI load because it uses a real 50 ms timer and asserts a wall-clock bound of 1 second. Schedule the rejection on the next turn and rely on the `/lost answer/` assertion; a regression that waits for the 60-second start timeout will still exceed the test timeout.</comment>
<file context>
@@ -404,14 +409,32 @@ test('a failed start is stopped before its failure reaches the caller', async ()
await settle();
- provider.failStart(0, new Error('lost answer'));
+ const startedAt = Date.now();
+ setTimeout(() => provider.failStart(0, new Error('lost answer')), 50);
await assert.rejects(allocating, /lost answer/);
+ assert.ok(Date.now() - startedAt < 1_000, 'the failure is not held to the start timeout');
</file context>
| */ | ||
| startTimeoutMs: number; | ||
| /** | ||
| * How long a teardown waits for one stop; a later teardown joins a stop that outlived it. Unset waits. |
There was a problem hiding this comment.
P2: The condensed stopTimeoutMs doc drops the note that the table does not bound findByLease. The table awaits findByLease with no timeout, so a driver author could leave it unbounded and hang release. Restore that sentence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provision-kit/src/provider-lease-table.ts, line 64:
<comment>The condensed `stopTimeoutMs` doc drops the note that the table does not bound `findByLease`. The table awaits `findByLease` with no timeout, so a driver author could leave it unbounded and hang release. Restore that sentence.</comment>
<file context>
@@ -70,9 +61,7 @@ export type ProviderLeaseDriver<Session, Handle, Request> = Readonly<{
- * How long one teardown waits for a stop to answer. A stop that outlives it is reported as unconfirmed,
- * and the next teardown joins the same stop instead of sending a second one. Unset waits for the answer.
- * Only the stop is bounded here; `findByLease` answers within the provider's own request budget.
+ * How long a teardown waits for one stop; a later teardown joins a stop that outlived it. Unset waits.
*/
stopTimeoutMs?: number;
</file context>
| * How long a teardown waits for one stop; a later teardown joins a stop that outlived it. Unset waits. | |
| * How long a teardown waits for one stop. A stop that outlives it is reported as unconfirmed, and the next | |
| * teardown joins that stop instead of sending a second one. Unset waits. Only the stop is bounded here; | |
| * `findByLease` answers within the provider's own request budget. |
Summary
Revyl's lease bookkeeping moves to a shared table,
@agent-device/provision-kit/provider-lease-table. Revyl supplies start, stop, find-by-lease, a rental key and errors.providerRuntimeInstance(provider, fields)joinsproviderRuntimeOwner; digests are unchanged. ADRs 0007 and 0019 record this. 22 files, 27 self-contained commits.The table owns:
starting→ready→ending) with a generation, re-checked after every await; one start per lease id.Revyl changes:
provider-lease-releasing), not handed a stopped session.Limrun moves onto the table in a follow-up now #4141 merged (Refs, not Fixes). Fallow's one clone group (platform-loader glue) predates this PR and sits in Limrun's
runtime.ts, which #4141 rewrote.Refs #4102
Validation
Head
dce5c56de0:check:affected --run --head dce5c56de0passed,exactCommit=true, receiptrun-a1swS8. Local Fallow audit: 0 findings.🤖 Generated with Claude Code