From 2b244394866bd81421051485d47a28bf06d4021c Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 14 Sep 2026 12:32:58 +0200 Subject: [PATCH 1/2] feat(core): create a new device identity for each lease under lease.identity fresh Adds lease.identity.{ios,android} (default reusable). A device records the policy it was created under. A fresh device serves one lease: its lease end is a shutdown and a delete, never an erase; a failed delete quarantines it and quarantine retries the delete. Startup finishes deleting spent devices left reclaiming or shutdown. Adds the shutdown -> quarantined transition. Closes #75 --- docs/CLI.md | 4 +- docs/CONFIGURATION.md | 8 +- docs/EVENTS.md | 14 +- docs/internal/ARCHITECTURE.md | 18 +- docs/internal/EVENTS.md | 14 +- src/cli/index.test.ts | 6 +- src/contract/operations.test.ts | 2 +- src/contract/schemas.test.ts | 3 + src/contract/schemas.ts | 7 + src/core/acquisition-planner.test.ts | 37 +- src/core/acquisition-planner.ts | 5 + src/core/cleanup/idle-destroy.test.ts | 6 +- src/core/cleanup/idle-shutdown.test.ts | 6 +- src/core/config.test.ts | 41 ++- src/core/config.ts | 16 + src/core/doctor.test.ts | 6 +- src/core/domain.test.ts | 16 +- src/core/domain.ts | 33 +- .../lease-acquisition-coordinator.test.ts | 2 +- src/core/lease-engine.test.ts | 334 +++++++++++++++++- src/core/lease-engine.ts | 14 + src/core/lease-health-monitor.test.ts | 2 +- src/core/nuke.test.ts | 6 +- src/core/quarantine-coordinator.test.ts | 68 +++- src/core/quarantine-coordinator.ts | 35 +- src/core/reaper.test.ts | 2 +- src/core/registry.test.ts | 162 ++++++++- src/core/registry.ts | 67 +++- src/core/startup-converger.test.ts | 36 ++ src/core/startup-converger.ts | 29 +- src/core/warm-pool-coordinator.test.ts | 119 ++++++- src/core/warm-pool-coordinator.ts | 100 +++++- src/daemon/dispatcher.test.ts | 7 +- src/daemon/main.ts | 1 + src/daemon/server.test.ts | 1 + src/gateway/dispatcher.test.ts | 6 +- src/http/test-fakes.ts | 1 + 37 files changed, 1137 insertions(+), 97 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index f091c4d..b2046b9 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -851,7 +851,9 @@ schema's source of truth. Human and JSON status include derived warm counts globally and per platform. `ready` devices contribute to those counts; `reclaiming` and `quarantined` devices remain visible as busy running capacity and never contribute to warm -inventory. A `quarantined` device is one whose release-time purge failed, or +inventory. A `quarantined` device is one whose release-time purge failed, one +created under `lease.identity` `fresh` whose lease-end delete failed (it is +retried as a delete, never returned to the pool), or one whose `provisioning`/`reclaiming` transition stalled past its driver-derived threshold (see `simlock doctor` below): it stays visible in `status` and `list --devices` with that state while `QuarantineCoordinator` retries it in diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index ed34f7c..b9deb2d 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -11,12 +11,14 @@ a warning. Inspect the effective, merged configuration at any time with | `capacity.strategy` | Which policy decides how many devices may exist and run at once: `resource` or `fixed`. The options under `capacity.config` are that strategy's own -- see [Capacity strategies](#capacity-strategies). | `resource` | | `idle.shutdownAfterMs` | How long an unused device sits idle before Simlock shuts it down (tier 1, reclaims RAM). | `10 minutes` | | `idle.deleteAfterMs` | How long a shut-down device sits idle before Simlock deletes it (tier 2, reclaims disk). | `1 hour` | -| `warmPool.quarantine.maxRetries` | Failed purge retries allowed on a quarantined device (after the triggering failure) before Simlock gives up and destroys it. | `3` | +| `warmPool.quarantine.maxRetries` | Failed purge retries allowed on a quarantined device (after the triggering failure) before Simlock gives up and destroys it. A device created under `lease.identity` `fresh` retries its delete instead of a purge. | `3` | | `warmPool.quarantine.retryBackoffMs` | Delay before the first quarantine purge retry. | `30 seconds` | | `warmPool.quarantine.retryBackoffMultiplier` | Growth factor applied to the backoff after each failed retry. | `2` | | `warmPool.quarantine.maxRetryBackoffMs` | Cap on the quarantine retry backoff. | `5 minutes` | | `lease.defaultTtlMs` | TTL applied to a lease whose `lease.request` carried no `ttlMs` — **that request only**. It is *not* the renew fallback: a renew given no explicit TTL re-applies the lease's own stored width, so a lease granted for longer keeps it. A lease not renewed before its deadline expires and its device is reclaimed. | `15 minutes` | | `lease.maxTtlMs` | Largest TTL a request or a renew may ask for. A larger `ttlMs` is rejected with `BAD_REQUEST` rather than silently clamped, so a caller is never left believing it has more time than it does. | `4 hours` | +| `lease.identity.ios` | Whether iOS leases reuse simulators. `reusable` erases a released simulator and returns it to the pool. `fresh` creates a simulator for each lease and deletes it when the lease ends — by release, by expiry, or because the device was lost — so no simulator serves two leases. If the delete fails, the simulator is quarantined, never leased again, and the delete is retried. **Boot cost:** under `fresh`, no simulator stays warm after a release, so every lease pays a full boot (about 30 seconds on an Apple silicon Mac). Each device keeps the setting it was created with, even if you change this value later. | `reusable` | +| `lease.identity.android` | The same setting for Android emulators. `fresh` is accepted, but it is designed for iOS. | `reusable` | | `gateway.url` | **Worker side.** Base URL of the gateway this worker joins, e.g. `wss://gw.example:4700`; the worker dials `/v1/uplink` and upgrades it to a WebSocket. Joining grants that gateway the `admin` role on this daemon, and `gateway.token` rides on the upgrade request as a bearer credential, so use `wss://` — or plain `ws://` only over loopback or inside your own tunnel. `http://` and `https://` are rejected at load. Unset means "do not join a fleet": the default, and the only thing that changes about a joined worker. | unset | | `gateway.token` | **Worker side.** The join token (`simlock token create --role worker`, minted on the gateway) this worker presents when it opens its uplink. Required whenever `gateway.url` is set. | unset | | `gateway.label` | **Worker side.** Display name for this worker in `simlock worker list`, `status`, the console, and on the lease's `worker` block. Display-only: nothing routes on it and it need not be unique. | the worker's own id | @@ -83,6 +85,10 @@ positive numbers. **`mode: "gateway"` with `http.enabled: false` is rejected at load**, naming the key: a gateway is the fleet's contact point over HTTP, so one nothing can reach has no safe reading. +`lease.identity.ios` and `lease.identity.android` must each be `reusable` or +`fresh`. Any other value is rejected at load, and the error names the key. +No lease request can change this setting. + `lease.defaultTtlMs` and `lease.maxTtlMs` must be positive numbers, and `lease.defaultTtlMs` must be `<=` `lease.maxTtlMs`. A config that violates either rule is **rejected at load and the daemon does not start**, naming the diff --git a/docs/EVENTS.md b/docs/EVENTS.md index 0fc989a..70a6399 100644 --- a/docs/EVENTS.md +++ b/docs/EVENTS.md @@ -31,14 +31,14 @@ the gateway's own lease index before any worker is ever contacted. |---|---|---|---|---| | `device.provisioned` | device id, spec, driver, duration | driver `provision` committed to registry | Registry | implemented | | `device.ready` | device id, boot duration | readiness probe passed | Registry | implemented | -| `device.reclaimed` | device id, strategy (erase/snapshot/wipe), duration | fresh-state reclaim finished | Registry | implemented | -| `device.purge-failed` | device id, lease id, attempted strategy, duration, stable error summary | release-time purge failed; the device enters `quarantined` (see below) rather than rejoining the pool | WarmPoolCoordinator | implemented | -| `device.quarantined` | device id, max retries, next retry deadline | a device committed to `quarantined` — present in the registry, still counted as running, not eligible for a grant. Fires immediately after `device.purge-failed` for a release-time purge failure, or on its own for a stalled-transition timeout (see `device.stalled-transition-detected`) | QuarantineCoordinator | implemented | -| `device.quarantine-recovered` | device id, attempts, reclaim strategy | a quarantined device's retried purge succeeded; it returned to `ready`/`shutdown` and rejoined the warm pool | QuarantineCoordinator | implemented | -| `device.quarantine-abandoned` | device id, attempts | a quarantined device exhausted its configured retry budget (`warmPool.quarantine.maxRetries`) and was destroyed | QuarantineCoordinator | implemented | -| `device.quarantine-stranded` | device id, attempts, stable error summary | a quarantined device exhausted its retry budget and the destroy that should have retired it also failed; it stays `quarantined` with no further retry until an operator intervenes | QuarantineCoordinator | implemented | +| `device.reclaimed` | device id, strategy (erase/snapshot/wipe), duration | fresh-state reclaim finished. Never emitted for a device created under `lease.identity` `fresh`: nothing is reclaimed, the device is deleted instead | Registry | implemented | +| `device.purge-failed` | device id, lease id, attempted strategy (erase/snapshot/wipe/delete), duration, stable error summary | release-time purge failed, or (strategy `delete`) the shutdown or delete that ends a `fresh` device's lease failed; the device enters `quarantined` (see below) rather than rejoining the pool. The strategy list can grow: a consumer must tolerate a strategy it does not know | WarmPoolCoordinator | implemented | +| `device.quarantined` | device id, max retries, next retry deadline | a device committed to `quarantined` — present in the registry, still counted as running, not eligible for a grant. Fires immediately after `device.purge-failed` for a release-time purge or delete failure, or on its own for a stalled-transition timeout (see `device.stalled-transition-detected`) | QuarantineCoordinator | implemented | +| `device.quarantine-recovered` | device id, attempts, reclaim strategy | a quarantined device's retried purge succeeded; it returned to `ready`/`shutdown` and rejoined the warm pool. Never emitted for a `fresh` device: its retry is a delete, and a successful one emits `device.deleted` | QuarantineCoordinator | implemented | +| `device.quarantine-abandoned` | device id, attempts | a quarantined device exhausted its configured retry budget (`warmPool.quarantine.maxRetries`) — purge retries, or delete retries for a `fresh` device — and was destroyed | QuarantineCoordinator | implemented | +| `device.quarantine-stranded` | device id, attempts, stable error summary | a quarantined device exhausted its retry budget (purge or delete retries) and the destroy that should have retired it also failed; it stays `quarantined` with no further retry until an operator intervenes | QuarantineCoordinator | implemented | | `device.shutdown` | device id, initiator (rule/command) | device stopped, still on disk | Registry; WarmPoolCoordinator for interrupted reclaim recovery | implemented | -| `device.deleted` | device id, initiator | device removed from disk and registry | Registry | implemented | +| `device.deleted` | device id, initiator (`lease-end` when a `fresh` device's lease ended and its delete completed, including a delete retried from quarantine) | device removed from disk and registry | Registry | implemented | | `device.foreign-state-detected` | device id, platform, expected (running/stopped), observed (running/stopped) | doctor reconcile found a managed device's observed boot state disagreeing with the committed registry state | Doctor | implemented | | `device.foreign-provenance-detected` | device id, platform, detail (erased/mark-mismatch/durable-mark-missing) | doctor reconcile found a managed device's provenance marks no longer proving Simlock owns it | Doctor | implemented | | `device.stalled-transition-detected` | device id, platform, state (provisioning/reclaiming), age, threshold | doctor reconcile found a `provisioning`/`reclaiming` device whose time in that state exceeds a driver-derived threshold — the driver call meant to resolve the transition never did | Doctor | implemented | diff --git a/docs/internal/ARCHITECTURE.md b/docs/internal/ARCHITECTURE.md index 592a84d..4fb88e9 100644 --- a/docs/internal/ARCHITECTURE.md +++ b/docs/internal/ARCHITECTURE.md @@ -879,12 +879,19 @@ One shared lifecycle for both platforms; drivers map onto it, never extend it: ``` provisioning → ready → leased → reclaiming → ready/shutdown → deleted - ↓ ↓ - └──────────→ quarantined ←─────┘ + ↓ ↓ ↓ + └──────────→ quarantined ←─────┴───────────────┘ ↓ ready/shutdown/deleted ``` +A device created under `lease.identity: fresh` serves one lease. Its lease end +skips the purge: `reclaiming → shutdown` (driver shutdown), then +`shutdown → deleted` (driver destroy). `mayBeGranted` in `domain.ts` keeps a +spent fresh device out of every grant path, including in the window between +those two commits. A failed delete enters quarantine from `shutdown`, and +quarantine retries the delete, never a reclaim. + All transitions go through the core. `simlock status` reads identically for iOS and Android because of this. @@ -905,9 +912,10 @@ helpers select targets by exact state (`state === "ready"`), never by excluding known-bad states. Anything that needs "in the registry, counts against capacity, not grantable" is expressed by adding its own entry into `quarantined`, not by inventing a second state: the release-time purge -failure (`reclaiming → quarantined`) and the stalled-transition timeout -(`provisioning → quarantined`, both owned by `QuarantineCoordinator`) are its -two entries. The latter fires from `simlock doctor`'s `stalled-transition` +failure (`reclaiming → quarantined`), a fresh device's failed delete +(`shutdown → quarantined`), and the stalled-transition timeout +(`provisioning → quarantined`, all owned by `QuarantineCoordinator`) are its +three entries. The latter fires from `simlock doctor`'s `stalled-transition` finding — a `provisioning`/`reclaiming` device whose time in that state has outrun a driver-derived threshold, meaning the driver call meant to resolve it never did and the registry's view has diverged from the driver's. Safer diff --git a/docs/internal/EVENTS.md b/docs/internal/EVENTS.md index e522b5e..14c51d2 100644 --- a/docs/internal/EVENTS.md +++ b/docs/internal/EVENTS.md @@ -44,14 +44,14 @@ worker is ever contacted. |---|---|---|---|---| | `device.provisioned` | device id, spec, driver, duration | driver `provision` committed to registry | Registry | implemented | | `device.ready` | device id, boot duration | readiness probe passed | Registry | implemented | -| `device.reclaimed` | device id, strategy (erase/snapshot/wipe), duration | fresh-state reclaim finished | Registry | implemented | -| `device.purge-failed` | device id, lease id, attempted strategy, duration, stable error summary | release-time purge failed; the device enters `quarantined` (see below) rather than rejoining the pool | WarmPoolCoordinator | implemented | -| `device.quarantined` | device id, max retries, next retry deadline | a device committed to `quarantined` — present in the registry, still counted as running, not eligible for a grant. Fires immediately after `device.purge-failed` for a release-time purge failure, or on its own for a stalled-transition timeout (see `device.stalled-transition-detected`) | QuarantineCoordinator | implemented | -| `device.quarantine-recovered` | device id, attempts, reclaim strategy | a quarantined device's retried purge succeeded; it returned to `ready`/`shutdown` and rejoined the warm pool | QuarantineCoordinator | implemented | -| `device.quarantine-abandoned` | device id, attempts | a quarantined device exhausted its configured retry budget (`warmPool.quarantine.maxRetries`) and was destroyed | QuarantineCoordinator | implemented | -| `device.quarantine-stranded` | device id, attempts, stable error summary | a quarantined device exhausted its retry budget and the destroy that should have retired it also failed; it stays `quarantined` with no further retry until an operator intervenes | QuarantineCoordinator | implemented | +| `device.reclaimed` | device id, strategy (erase/snapshot/wipe), duration | fresh-state reclaim finished. Never emitted for a device created under `lease.identity` `fresh` (#75): nothing is reclaimed, the device is deleted instead | Registry | implemented | +| `device.purge-failed` | device id, lease id, attempted strategy (erase/snapshot/wipe/delete), duration, stable error summary | release-time purge failed, or (strategy `delete`, #75) the shutdown or delete that ends a `fresh` device's lease failed; the device enters `quarantined` (see below) rather than rejoining the pool. `delete` widens a published vocabulary (events rule 6 allows additive changes): a consumer must tolerate a strategy it does not know | WarmPoolCoordinator (strategy `delete`: WarmPoolCoordinator's spent-device path) | implemented | +| `device.quarantined` | device id, max retries, next retry deadline | a device committed to `quarantined` — present in the registry, still counted as running, not eligible for a grant. Fires immediately after `device.purge-failed` for a release-time purge or delete failure (`reclaiming`/`shutdown → quarantined`), or on its own for a stalled-transition timeout (see `device.stalled-transition-detected`) | QuarantineCoordinator | implemented | +| `device.quarantine-recovered` | device id, attempts, reclaim strategy | a quarantined device's retried purge succeeded; it returned to `ready`/`shutdown` and rejoined the warm pool. Never emitted for a spent `fresh` device (`mayBeGranted` false): its retry is a delete, and a successful one emits `device.deleted` with initiator `lease-end` | QuarantineCoordinator | implemented | +| `device.quarantine-abandoned` | device id, attempts | a quarantined device exhausted its configured retry budget (`warmPool.quarantine.maxRetries`) — purge retries, or delete retries for a spent `fresh` device — and was destroyed | QuarantineCoordinator | implemented | +| `device.quarantine-stranded` | device id, attempts, stable error summary | a quarantined device exhausted its retry budget (purge or delete retries) and the destroy that should have retired it also failed; it stays `quarantined` with no further retry until an operator intervenes | QuarantineCoordinator | implemented | | `device.shutdown` | device id, initiator (rule/command) | device stopped, still on disk | Registry; WarmPoolCoordinator for interrupted reclaim recovery | implemented | -| `device.deleted` | device id, initiator | device removed from disk and registry | Registry | implemented | +| `device.deleted` | device id, initiator (`lease-end` when a `fresh` device's lease ended and its delete completed — from WarmPoolCoordinator, at startup convergence, or retried from quarantine; #75) | device removed from disk and registry | Registry | implemented | | `device.foreign-state-detected` | device id, platform, expected (running/stopped), observed (running/stopped) | doctor reconcile found a managed device's observed boot state disagreeing with the committed registry state | Doctor | implemented | | `device.foreign-provenance-detected` | device id, platform, detail (erased/mark-mismatch/durable-mark-missing) | doctor reconcile found a managed device's provenance marks no longer proving Simlock owns it | Doctor | implemented | | `device.stalled-transition-detected` | device id, platform, state (provisioning/reclaiming), age, threshold | doctor reconcile found a `provisioning`/`reclaiming` device whose time in that state exceeds a driver-derived threshold (`stalledTransition.thresholdMultiplier` over `Driver.estimate`, floored at `stalledTransition.minimumThresholdMs`) — the driver call meant to resolve the transition never did | Doctor | implemented | diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index 28536a9..61a76d2 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -3327,7 +3327,11 @@ function testConfig(): Config { http: { enabled: false, host: "127.0.0.1", port: 4700 }, ios: { slim: { enabled: false, bootTimeoutMs: 600_000 } }, idle: { deleteAfterMs: 60_000, shutdownAfterMs: 10_000 }, - lease: { defaultTtlMs: 60_000, maxTtlMs: 3_600_000 }, + lease: { + defaultTtlMs: 60_000, + maxTtlMs: 3_600_000, + identity: { ios: "reusable", android: "reusable" }, + }, capacity: { strategy: "resource", config: { diff --git a/src/contract/operations.test.ts b/src/contract/operations.test.ts index 9b6f78a..891088a 100644 --- a/src/contract/operations.test.ts +++ b/src/contract/operations.test.ts @@ -394,7 +394,7 @@ describe("operation input/output round trips", () => { maxRetryBackoffMs: 1, }, }, - lease: { defaultTtlMs: 1, maxTtlMs: 1 }, + lease: { defaultTtlMs: 1, maxTtlMs: 1, identity: { ios: "fresh", android: "reusable" } }, exec: { timeoutMs: 1 }, diskPressure: { freeBytesThreshold: 1 }, eventBuffer: { capacity: 1 }, diff --git a/src/contract/schemas.test.ts b/src/contract/schemas.test.ts index a62c64e..2544743 100644 --- a/src/contract/schemas.test.ts +++ b/src/contract/schemas.test.ts @@ -28,6 +28,7 @@ describe("leaseGrantSchema's device projection", () => { "quarantinedAt", "quarantineAttempts", "quarantineNextRetryAt", + "leaseIdentity", "transitionAgeMs", ] as const; @@ -51,6 +52,7 @@ describe("leaseGrantSchema's device projection", () => { quarantineNextRetryAt: 60, address: "127.0.0.1:1234", featureProfile: "reduced", + leaseIdentity: "fresh", transitionAgeMs: 70, }; } @@ -137,6 +139,7 @@ describe("statusDeviceSchema's device projection", () => { quarantineNextRetryAt: 60, address: "127.0.0.1:1234", featureProfile: "reduced", + leaseIdentity: "fresh", transitionAgeMs: 70, }; } diff --git a/src/contract/schemas.ts b/src/contract/schemas.ts index 165f58c..df40a37 100644 --- a/src/contract/schemas.ts +++ b/src/contract/schemas.ts @@ -34,6 +34,8 @@ const deviceStateSchema = z.enum([ const featureProfileSchema = z.enum(["full", "reduced"]); +const leaseIdentitySchema = z.enum(["reusable", "fresh"]); + const deviceSpecSchema = z.object({ platform: platformSchema, model: z.string(), @@ -60,6 +62,7 @@ export const deviceRecordSchema = z.object({ quarantineNextRetryAt: z.number().optional(), address: z.string().optional(), featureProfile: featureProfileSchema.optional(), + leaseIdentity: leaseIdentitySchema.optional(), /** Decoration added by `status.get`/`list.get`; absent for a device not mid-transition. */ transitionAgeMs: z.number().optional(), }); @@ -433,6 +436,10 @@ export const configSchema = z.object({ lease: z.object({ defaultTtlMs: z.number(), maxTtlMs: z.number(), + identity: z.object({ + ios: leaseIdentitySchema, + android: leaseIdentitySchema, + }), }), exec: z.object({ timeoutMs: z.number() }), diskPressure: z.object({ freeBytesThreshold: z.number() }), diff --git a/src/core/acquisition-planner.test.ts b/src/core/acquisition-planner.test.ts index 83b0868..9b01e55 100644 --- a/src/core/acquisition-planner.test.ts +++ b/src/core/acquisition-planner.test.ts @@ -42,7 +42,7 @@ const config: Config = { retryBackoffMultiplier: 2, }, }, - lease: { defaultTtlMs: 100, maxTtlMs: 100 }, + lease: { defaultTtlMs: 100, maxTtlMs: 100, identity: { ios: "reusable", android: "reusable" } }, capacity: { strategy: "resource", config: { @@ -194,4 +194,39 @@ describe("AcquisitionPlanner", () => { kind: "no-capacity", }); }); + + it("never selects a spent fresh device from the ready scan or from the shutdown scan", () => { + const unused = (state: "ready" | "shutdown") => ({ + ...device(`unused-${state}`, state), + leaseIdentity: "fresh" as const, + }); + const spent = (state: "ready" | "shutdown") => ({ ...unused(state), lastLeaseEndedAt: 5 }); + + // Control: the same fresh records that have not served a lease yet are selected, so what + // turns the spent ones away below is the lease they already served. + expect(plan(planner().planner, [unused("ready")])).toMatchObject({ kind: "grant-ready" }); + expect(plan(planner().planner, [unused("shutdown")])).toMatchObject({ kind: "boot-shutdown" }); + + // ios.maxDevices is 1 here, so the spent device is also the only managed eviction victim. + expect(plan(planner().planner, [spent("ready")])).toMatchObject({ kind: "evict-managed" }); + expect(plan(planner().planner, [spent("shutdown")])).toMatchObject({ kind: "evict-managed" }); + }); + + it("counts a spent fresh device against managed-device capacity until it is deleted", () => { + const spent = { + ...device("spent", "shutdown"), + lastLeaseEndedAt: 5, + leaseIdentity: "fresh" as const, + }; + + // At ios.maxDevices 1, the spent device fills the only slot: no provision is planned. + expect(plan(planner().planner, [spent])).toMatchObject({ + device: spent, + kind: "evict-managed", + }); + + const result = plan(planner().planner, [{ ...spent, state: "deleted" }]); + expect(result).toMatchObject({ kind: "provision" }); + if (result.kind === "provision") result.reservation.release(); + }); }); diff --git a/src/core/acquisition-planner.ts b/src/core/acquisition-planner.ts index 6696a6c..56eeeb2 100644 --- a/src/core/acquisition-planner.ts +++ b/src/core/acquisition-planner.ts @@ -4,6 +4,7 @@ import { type DeviceRecord, type DeviceSpec, type LeaseRecord, + mayBeGranted, type Platform, sameSpec, } from "./domain.js"; @@ -58,14 +59,18 @@ export class AcquisitionPlanner { const ready = snapshot.devices.find( (device) => device.state === "ready" && + mayBeGranted(device) && !this.claims.isClaimed(device.id) && sameSpec(device.spec, spec), ); if (ready !== undefined) return { device: ready, kind: "grant-ready" }; + // A spent fresh device sits `shutdown` between its lease-end shutdown commit and its + // delete; `mayBeGranted` is what keeps it from being booted for a new lease in that window. const shutdown = snapshot.devices.find( (device) => device.state === "shutdown" && + mayBeGranted(device) && !this.claims.isClaimed(device.id) && sameSpec(device.spec, spec), ); diff --git a/src/core/cleanup/idle-destroy.test.ts b/src/core/cleanup/idle-destroy.test.ts index c774235..8ec0e68 100644 --- a/src/core/cleanup/idle-destroy.test.ts +++ b/src/core/cleanup/idle-destroy.test.ts @@ -38,7 +38,11 @@ const config: Config = { retryBackoffMultiplier: 2, }, }, - lease: { defaultTtlMs: 60_000, maxTtlMs: 3_600_000 }, + lease: { + defaultTtlMs: 60_000, + maxTtlMs: 3_600_000, + identity: { ios: "reusable", android: "reusable" }, + }, capacity: { strategy: "resource", config: { diff --git a/src/core/cleanup/idle-shutdown.test.ts b/src/core/cleanup/idle-shutdown.test.ts index d7832b1..7a1189f 100644 --- a/src/core/cleanup/idle-shutdown.test.ts +++ b/src/core/cleanup/idle-shutdown.test.ts @@ -36,7 +36,11 @@ const config: Config = { retryBackoffMultiplier: 2, }, }, - lease: { defaultTtlMs: 60_000, maxTtlMs: 3_600_000 }, + lease: { + defaultTtlMs: 60_000, + maxTtlMs: 3_600_000, + identity: { ios: "reusable", android: "reusable" }, + }, capacity: { strategy: "resource", config: { diff --git a/src/core/config.test.ts b/src/core/config.test.ts index a049dc4..f9a5dc3 100644 --- a/src/core/config.test.ts +++ b/src/core/config.test.ts @@ -88,7 +88,11 @@ describe("loadConfig", () => { maxConcurrentRecoveries: 1, }, idle: { deleteAfterMs: 60 * 60_000, shutdownAfterMs: 10 * 60_000 }, - lease: { defaultTtlMs: 15 * 60_000, maxTtlMs: 4 * 60 * 60_000 }, + lease: { + defaultTtlMs: 15 * 60_000, + maxTtlMs: 4 * 60 * 60_000, + identity: { ios: "reusable", android: "reusable" }, + }, capacity: { strategy: "resource", config: { @@ -357,6 +361,35 @@ describe("loadConfig", () => { expect(config.lease).toMatchObject({ defaultTtlMs: 40_000, maxTtlMs: 40_000 }); }); + it.each([ + [{ lease: { identity: { ios: "disposable" } } }, "lease.identity.ios"], + [{ lease: { identity: { android: true } } }, "lease.identity.android"], + ])( + "rejects a lease.identity value other than reusable or fresh, naming the key (%#)", + async (contents, path) => { + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/agent/.simlock"); + await filesystem.writeFileAtomic(configPath, JSON.stringify(contents)); + + await expect( + loadConfig({ configPath, filesystem, systemStats: createStats() }), + ).rejects.toThrow(path); + }, + ); + + it("reads lease.identity per platform and leaves the unset platform reusable", async () => { + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/agent/.simlock"); + await filesystem.writeFileAtomic( + configPath, + JSON.stringify({ lease: { identity: { ios: "fresh" } } }), + ); + + const config = await loadConfig({ configPath, filesystem, systemStats: createStats() }); + + expect(config.lease.identity).toEqual({ android: "reusable", ios: "fresh" }); + }); + it.each([ [{ lease: { defaultTtlMs: 0 } }, "lease.defaultTtlMs"], [{ lease: { maxTtlMs: -1 } }, "lease.maxTtlMs"], @@ -420,7 +453,11 @@ describe("loadConfig", () => { expect(warn).toHaveBeenCalledWith(`Unknown config key: "lease.${retired}"`); // Not aliased onto anything: the new keys keep their own defaults. - expect(config.lease).toEqual({ defaultTtlMs: 15 * 60_000, maxTtlMs: 4 * 60 * 60_000 }); + expect(config.lease).toEqual({ + defaultTtlMs: 15 * 60_000, + identity: { android: "reusable", ios: "reusable" }, + maxTtlMs: 4 * 60 * 60_000, + }); }, ); diff --git a/src/core/config.ts b/src/core/config.ts index 125766d..f49f2af 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -11,6 +11,7 @@ import { type ResourceStrategyOptions, } from "./capacity/index.js"; import { resourceOptionValidators } from "./capacity/strategies/resource/index.js"; +import type { LeaseIdentity } from "./domain.js"; import { booleanValue, ConfigError, @@ -142,6 +143,15 @@ export interface Config { /** The largest TTL a request or a renew may ask for; more is `BAD_REQUEST`, never a * silent clamp, so a caller is never left believing it has more time than it does. */ readonly maxTtlMs: number; + /** + * The lease-identity policy per platform (see `LeaseIdentity`), read by key for a new + * device's own platform when it is registered. Daemon configuration only: no lease request + * can change it, and a device keeps the value it was created under after this changes. + */ + readonly identity: { + readonly ios: LeaseIdentity; + readonly android: LeaseIdentity; + }; }; /** * ADR 0005 §19e. Platform-agnostic on purpose: it bounds the *daemon's* willingness to wait @@ -567,6 +577,7 @@ function defaultConfig( lease: { defaultTtlMs: DEFAULT_LEASE_TTL_MS, maxTtlMs: DEFAULT_LEASE_MAX_TTL_MS, + identity: { ios: "reusable", android: "reusable" }, }, exec: { timeoutMs: DEFAULT_EXEC_TIMEOUT_MS }, diskPressure: { freeBytesThreshold: 10 * 1024 ** 3 }, @@ -646,6 +657,7 @@ function validateConfigLayer( const LOG_LEVELS: readonly LogLevel[] = ["debug", "info", "warn", "error"]; const DAEMON_MODES: readonly DaemonMode[] = ["worker", "gateway"]; const DOWNLOAD_POLICIES: readonly DownloadPolicy[] = ["never", "on-request", "always"]; +const LEASE_IDENTITIES: readonly LeaseIdentity[] = ["reusable", "fresh"]; /** * The `capacity.config` validator is the selected strategy's own, so a strategy @@ -692,6 +704,10 @@ function configValidators(strategy: CapacityStrategyName): Record stableObservations: 2, }, idle: { deleteAfterMs: 10, shutdownAfterMs: 5 }, - lease: { defaultTtlMs: 60_000, maxTtlMs: 3_600_000 }, + lease: { + defaultTtlMs: 60_000, + maxTtlMs: 3_600_000, + identity: { ios: "reusable", android: "reusable" }, + }, capacity: { strategy: "resource", config: { diff --git a/src/core/domain.test.ts b/src/core/domain.test.ts index 3ef4c73..9537d89 100644 --- a/src/core/domain.test.ts +++ b/src/core/domain.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { type DeviceRecord, IllegalTransition, transition, transitionEnteredAt } from "./index.js"; -import { type DeviceSpec, sameSpec } from "./domain.js"; +import { type DeviceSpec, mayBeGranted, sameSpec } from "./domain.js"; const baseDevice: Omit = { createdAt: 1_000, @@ -27,6 +27,7 @@ describe("transition", () => { ["quarantined", "deleted"], ["shutdown", "ready"], ["shutdown", "deleted"], + ["shutdown", "quarantined"], ] as const)("allows %s -> %s", (from, to) => { const result = transition({ ...baseDevice, state: from }, to); @@ -124,6 +125,19 @@ describe("transitionEnteredAt", () => { }); }); +describe("mayBeGranted", () => { + it("refuses only a fresh device that has ended a lease", () => { + const ready = { ...baseDevice, state: "ready" as const }; + + expect(mayBeGranted({ ...ready, leaseIdentity: "fresh", lastLeaseEndedAt: 2_000 })).toBe(false); + expect(mayBeGranted({ ...ready, leaseIdentity: "fresh" })).toBe(true); + expect(mayBeGranted({ ...ready, leaseIdentity: "reusable", lastLeaseEndedAt: 2_000 })).toBe( + true, + ); + expect(mayBeGranted({ ...ready, lastLeaseEndedAt: 2_000 })).toBe(true); + }); +}); + describe("sameSpec", () => { const spec: DeviceSpec = { model: "iPhone 16", osVersion: "26.5", platform: "ios" }; diff --git a/src/core/domain.ts b/src/core/domain.ts index 3c5f6c8..d016a84 100644 --- a/src/core/domain.ts +++ b/src/core/domain.ts @@ -28,6 +28,13 @@ export function sameSpec(left: DeviceSpec, right: DeviceSpec): boolean { ); } +/** + * Whether a device may serve more than one lease. `reusable` devices are purged and returned to + * the pool after each lease; a `fresh` device serves exactly one lease and is then deleted. Read + * from `lease.identity.` when the device is created and fixed on its record from then on. + */ +export type LeaseIdentity = "reusable" | "fresh"; + export type DeviceState = | "provisioning" | "ready" @@ -71,6 +78,22 @@ export interface DeviceRecord { * driver. */ readonly featureProfile?: "full" | "reduced"; + /** + * The lease-identity policy this device was created under (see `LeaseIdentity`). The registry + * stamps it on every device it registers and loads a record written before this field existed + * as `reusable`; absent reads the same way. + */ + readonly leaseIdentity?: LeaseIdentity; +} + +/** + * The one answer to "may this device be handed to a new lease?". A `fresh` device that has ended + * a lease is spent: `beginRelease` stamps `lastLeaseEndedAt` on every lease end, so that stamp is + * the proof it already served its one lease. Every other device is grantable as far as identity + * goes; state, spec, and claims are the caller's own checks. + */ +export function mayBeGranted(device: DeviceRecord): boolean { + return device.leaseIdentity !== "fresh" || device.lastLeaseEndedAt === undefined; } export interface LeaseRecord { @@ -110,9 +133,11 @@ export interface LeaseRecord { * right now, sitting outside the `ready`/`shutdown` states every grant and * eviction path already selects on. `reclaiming -> quarantined` is its release-time * purge-failure entry (see WarmPoolCoordinator); `provisioning -> quarantined` is its - * stalled-transition entry (Doctor, for a `provisioning` that never finished) -- its - * own entry into the same state rather than a second one. Exits are symmetric either - * way: `ready` on a successful retry, `shutdown`/`deleted` on giving up. + * stalled-transition entry (Doctor, for a `provisioning` that never finished); + * `shutdown -> quarantined` is a spent fresh device whose delete failed after its + * lease-end shutdown committed -- each its own entry into the same state rather than a + * second one. Exits: `ready` on a successful retry, `deleted` on a successful retried + * delete, `shutdown`/`deleted` on giving up. */ const legalTransitions: Readonly> = { provisioning: ["ready", "deleted", "quarantined"], @@ -120,7 +145,7 @@ const legalTransitions: Readonly> = leased: ["reclaiming"], reclaiming: ["ready", "shutdown", "quarantined"], quarantined: ["ready", "shutdown", "deleted"], - shutdown: ["ready", "deleted"], + shutdown: ["ready", "deleted", "quarantined"], deleted: [], }; diff --git a/src/core/lease-acquisition-coordinator.test.ts b/src/core/lease-acquisition-coordinator.test.ts index 6ba0211..f4e6d7c 100644 --- a/src/core/lease-acquisition-coordinator.test.ts +++ b/src/core/lease-acquisition-coordinator.test.ts @@ -49,7 +49,7 @@ function config(maxDevices = 1): Config { }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, idle: { deleteAfterMs: 60_000, shutdownAfterMs: 10_000 }, - lease: { defaultTtlMs: 100, maxTtlMs: 100 }, + lease: { defaultTtlMs: 100, maxTtlMs: 100, identity: { ios: "reusable", android: "reusable" } }, capacity: { strategy: "resource", config: { diff --git a/src/core/lease-engine.test.ts b/src/core/lease-engine.test.ts index b4c527c..0201655 100644 --- a/src/core/lease-engine.test.ts +++ b/src/core/lease-engine.test.ts @@ -46,7 +46,12 @@ function config(overrides: Partial = {}): Config { }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, idle: { deleteAfterMs: 60_000, shutdownAfterMs: 10_000 }, - lease: { defaultTtlMs: 100, maxTtlMs: 14_400_000, ...overrides }, + lease: { + defaultTtlMs: 100, + maxTtlMs: 14_400_000, + identity: { ios: "reusable", android: "reusable" }, + ...overrides, + }, capacity: { strategy: "resource", config: { @@ -80,6 +85,7 @@ async function createHarness( options: { readonly driver?: FakeDriver; readonly drivers?: readonly FakeDriver[]; + readonly identity?: Config["lease"]["identity"]; readonly lease?: Partial; readonly limits?: CapacityLimits; } = {}, @@ -95,9 +101,13 @@ async function createHarness( eventBus: bus, filesystem, idGenerator: { generate: () => `${nextId++}` }, + ...(options.identity === undefined ? {} : { leaseIdentity: options.identity }), statePath, }); - const baseConfig = config(options.lease); + const baseConfig = config({ + ...options.lease, + ...(options.identity === undefined ? {} : { identity: options.identity }), + }); const engineConfig: Config = options.limits === undefined ? baseConfig @@ -1438,3 +1448,323 @@ describe("LeaseEngine startup reclaim backgrounding (#43)", () => { expect(harness.registry.snapshot.leases).toHaveLength(1); }); }); + +describe("LeaseEngine fresh lease identity (#75)", () => { + const freshIos = { android: "reusable", ios: "fresh" } as const; + + function driverDeviceIdOf( + harness: { readonly registry: Registry }, + deviceId: string, + ): string | undefined { + return harness.registry.snapshot.devices.find((device) => device.id === deviceId) + ?.driverDeviceId; + } + + function destroyedDriverDeviceIds(driver: FakeDriver): string[] { + return driver.calls + .filter((call) => call.operation === "destroy") + .map((call) => (call.arguments[0] as { readonly deviceId: string }).deviceId); + } + + it("gives two sequential fresh leases for one shape different driver device ids", async () => { + const harness = await createHarness({ identity: freshIos }); + + const first = await harness.engine.request(request, { ownerId: "first", requesterId: "first" }); + await harness.engine.release(first.lease.id, "explicit"); + await harness.engine.settle(); + const second = await harness.engine.request(request, { + ownerId: "second", + requesterId: "second", + }); + + expect(second.device.id).not.toBe(first.device.id); + expect(driverDeviceIdOf(harness, second.device.id)).not.toBe( + driverDeviceIdOf(harness, first.device.id), + ); + expect(harness.driver.calls.filter((call) => call.operation === "provision")).toHaveLength(2); + }); + + it("takes a released fresh device to deleted and destroys its driver device, without an erase", async () => { + const harness = await createHarness({ identity: freshIos }); + const granted = await harness.engine.request(request, { ownerId: "a", requesterId: "a" }); + const driverDeviceId = driverDeviceIdOf(harness, granted.device.id); + + await harness.engine.release(granted.lease.id, "explicit"); + await harness.engine.settle(); + + expect(harness.registry.snapshot.devices).toMatchObject([ + { id: granted.device.id, state: "deleted" }, + ]); + expect(destroyedDriverDeviceIds(harness.driver)).toEqual([driverDeviceId]); + expect(harness.driver.calls.map((call) => call.operation)).not.toContain("reclaim"); + expect(harness.bus.replay().filter((event) => event.event === "device.deleted")).toMatchObject([ + { payload: { deviceId: granted.device.id, initiator: "lease-end" } }, + ]); + expect(harness.bus.replay().map((event) => event.event)).not.toContain("device.reclaimed"); + }); + + it("deletes the device of an expired fresh lease", async () => { + const harness = await createHarness({ identity: freshIos, lease: { defaultTtlMs: 10 } }); + const granted = await harness.engine.request(request, { ownerId: "a", requesterId: "a" }); + + harness.clock.advance(10); + await flush(); + await harness.engine.settle(); + + expect(harness.bus.replay().map((event) => event.event)).toContain("lease.expired"); + expect(harness.registry.snapshot.devices).toMatchObject([ + { id: granted.device.id, state: "deleted" }, + ]); + expect(destroyedDriverDeviceIds(harness.driver)).toHaveLength(1); + }); + + it("deletes the device of a recovery-driven lease termination", async () => { + const harness = await createHarness({ + identity: freshIos, + lease: { defaultTtlMs: 14_400_000 }, + }); + const granted = await harness.engine.request(request, { ownerId: "a", requesterId: "a" }); + // The device vanished from driver reality: recovery gives up and ends the lease as lost. + harness.driver.setManagedReality({ devices: [], processes: [] }); + + harness.engine.healthMonitor.start(); + for (let tick = 0; tick < 3; tick += 1) { + harness.clock.advance(30_000); + await flush(); + } + await harness.engine.settle(); + harness.engine.healthMonitor.dispose(); + + expect(harness.bus.replay().filter((event) => event.event === "lease.released")).toMatchObject([ + { payload: { leaseId: granted.lease.id, reason: "device-lost" } }, + ]); + expect(harness.registry.snapshot.devices).toMatchObject([ + { id: granted.device.id, state: "deleted" }, + ]); + }); + + it("quarantines a fresh device whose delete fails, keeps it ungrantable, and reports strategy delete", async () => { + const driver = new FakeDriver({ + availableOsVersions: ["26.5"], + clock: new FakeClock(1_000), + platform: "ios", + }); + driver.failOn("destroy", 1, new DriverCrashError("delete exploded")); + const harness = await createHarness({ + driver, + identity: freshIos, + limits: { + android: { maxDevices: 1, maxRunning: 1 }, + ios: { maxDevices: 2, maxRunning: 2 }, + maxRunning: 3, + }, + }); + const first = await harness.engine.request(request, { ownerId: "first", requesterId: "first" }); + + await harness.engine.release(first.lease.id, "explicit"); + await harness.engine.settle(); + + expect(harness.registry.snapshot.devices).toMatchObject([ + { id: first.device.id, state: "quarantined" }, + ]); + expect( + harness.bus.replay().filter((event) => event.event === "device.purge-failed"), + ).toMatchObject([ + { + payload: { + attemptedStrategy: "delete", + deviceId: first.device.id, + error: "DriverCrashError: delete exploded", + leaseId: first.lease.id, + }, + }, + ]); + const second = await harness.engine.request(request, { + noWait: true, + ownerId: "second", + requesterId: "second", + }); + expect(second.device.id).not.toBe(first.device.id); + }); + + it("still deletes a device created under fresh after the configuration changes to reusable", async () => { + const filesystem = new MemoryFilesystem(); + const clock = new FakeClock(1_000); + const bus = new EventBus(clock); + let nextId = 1; + const idGenerator = { generate: () => `${nextId++}` }; + const driver = new FakeDriver({ availableOsVersions: ["26.5"], clock, platform: "ios" }); + const systemStats = new FakeSystemStats({ + cpuCount: 8, + freeRamBytes: 32 * gibibyte, + totalRamBytes: 32 * gibibyte, + }); + const before = new LeaseEngine({ + clock, + config: config({ identity: freshIos }), + drivers: [driver], + eventBus: bus, + idGenerator, + registry: await Registry.load({ + clock, + eventBus: bus, + filesystem, + idGenerator, + leaseIdentity: freshIos, + statePath, + }), + systemStats, + }); + const granted = await before.request(request, { ownerId: "a", requesterId: "a" }); + before.dispose(); + + // The operator switches iOS back to reusable and restarts the daemon mid-lease. + const reusable = { android: "reusable", ios: "reusable" } as const; + const registry = await Registry.load({ + clock, + eventBus: bus, + filesystem, + idGenerator, + leaseIdentity: reusable, + statePath, + }); + const after = new LeaseEngine({ + clock, + config: config({ identity: reusable, defaultTtlMs: 14_400_000 }), + drivers: [driver], + eventBus: bus, + idGenerator, + registry, + systemStats, + }); + await after.convergeRunningCapacity(); + await after.release(granted.lease.id, "explicit"); + await after.settle(); + after.dispose(); + + expect(registry.snapshot.devices).toMatchObject([ + { id: granted.device.id, leaseIdentity: "fresh", state: "deleted" }, + ]); + expect(driver.calls.map((call) => call.operation)).not.toContain("reclaim"); + }); + + it("still reclaims a device created under reusable and returns it to the pool", async () => { + const harness = await createHarness(); + const first = await harness.engine.request(request, { ownerId: "first", requesterId: "first" }); + + await harness.engine.release(first.lease.id, "explicit"); + await harness.engine.settle(); + const second = await harness.engine.request(request, { + ownerId: "second", + requesterId: "second", + }); + + expect(harness.registry.snapshot.devices).toMatchObject([ + { id: first.device.id, leaseIdentity: "reusable", state: "leased" }, + ]); + expect(second.device.id).toBe(first.device.id); + const operations = harness.driver.calls.map((call) => call.operation); + expect(operations.filter((operation) => operation === "reclaim")).toHaveLength(1); + expect(operations).not.toContain("destroy"); + }); + + /** + * The persisted state a daemon leaves behind when it dies part-way through a fresh device's + * lease end, built through the same registry calls that path makes, then read by a new process. + */ + async function restartAfterCrash(crashedIn: "reclaiming" | "shutdown") { + const filesystem = new MemoryFilesystem(); + const clock = new FakeClock(1_000); + const bus = new EventBus(clock); + let nextId = 1; + const idGenerator = { generate: () => `${nextId++}` }; + // One driver across both processes: the simulator outlives the daemon. + const driver = new FakeDriver({ availableOsVersions: ["26.5"], clock, platform: "ios" }); + const crashed = await Registry.load({ + clock, + eventBus: bus, + filesystem, + idGenerator, + leaseIdentity: freshIos, + statePath, + }); + const driverDevice = await driver.makeReady(await driver.provision(request)); + const device = await crashed.registerDevice({ + driverData: driverDevice.driverData, + driverDeviceId: driverDevice.deviceId, + provisionDuration: 0, + spec: request, + }); + await crashed.transitionDevice(device.id, "ready", { + event: "device.ready", + payload: { bootDuration: 0, deviceId: device.id }, + }); + const lease = await crashed.createLease({ + deviceId: device.id, + ownerId: "gone", + requesterId: "gone", + ttlDeadline: 60_000, + ttlMs: 60_000, + }); + await crashed.beginRelease(lease.id); + if (crashedIn === "shutdown") { + await driver.shutdown(driverDevice); + await crashed.completeReclaimWithoutPurge(device.id); + } + + const registry = await Registry.load({ + clock, + eventBus: bus, + filesystem, + idGenerator, + leaseIdentity: freshIos, + statePath, + }); + const engine = new LeaseEngine({ + clock, + config: config({ identity: freshIos }), + drivers: [driver], + eventBus: bus, + idGenerator, + registry, + systemStats: new FakeSystemStats({ + cpuCount: 8, + freeRamBytes: 32 * gibibyte, + totalRamBytes: 32 * gibibyte, + }), + }); + const operationsBeforeStart = driver.calls.length; + return { + device, + driver, + engine, + operationsSinceStart: () => + driver.calls.slice(operationsBeforeStart).map((call) => call.operation), + registry, + }; + } + + it("deletes a fresh device on the next start when the daemon crashed while it was reclaiming", async () => { + const restarted = await restartAfterCrash("reclaiming"); + expect(restarted.registry.snapshot.devices).toMatchObject([{ state: "reclaiming" }]); + + await restarted.engine.convergeRunningCapacity(); + + expect(restarted.registry.snapshot.devices).toMatchObject([ + { id: restarted.device.id, state: "deleted" }, + ]); + expect(restarted.operationsSinceStart()).toEqual(["shutdown", "destroy"]); + }); + + it("deletes a fresh device on the next start when the daemon crashed after its shutdown commit and before its delete", async () => { + const restarted = await restartAfterCrash("shutdown"); + expect(restarted.registry.snapshot.devices).toMatchObject([{ state: "shutdown" }]); + + await restarted.engine.convergeRunningCapacity(); + + expect(restarted.registry.snapshot.devices).toMatchObject([ + { id: restarted.device.id, state: "deleted" }, + ]); + expect(restarted.operationsSinceStart()).toEqual(["destroy"]); + }); +}); diff --git a/src/core/lease-engine.ts b/src/core/lease-engine.ts index 2dd2b01..25355cf 100644 --- a/src/core/lease-engine.ts +++ b/src/core/lease-engine.ts @@ -200,6 +200,20 @@ export class LeaseEngine { }, quarantineRestore: { restore: () => this.#quarantine.restore() }, registry: options.registry, + spentDeviceDeletion: { + // A failed delete must not stop the daemon from starting: the device stays `shutdown` + // and ungrantable, and the next start (or the idle delete rule) tries again. + deleteSpent: async (device) => { + try { + await this.#warmPool.deleteSpent(device.id); + } catch (error: unknown) { + options.logger?.error("startup delete of a spent device failed", { + deviceId: device.id, + error: error instanceof Error ? error.message : String(error), + }); + } + }, + }, timers: this.#leases, }); this.healthMonitor = new LeaseHealthMonitor({ diff --git a/src/core/lease-health-monitor.test.ts b/src/core/lease-health-monitor.test.ts index f421948..b0b223b 100644 --- a/src/core/lease-health-monitor.test.ts +++ b/src/core/lease-health-monitor.test.ts @@ -46,7 +46,7 @@ function config(overrides: Partial = {}): Config { ...overrides, }, idle: { deleteAfterMs: 30_000, shutdownAfterMs: 10_000 }, - lease: { defaultTtlMs: 100, maxTtlMs: 100 }, + lease: { defaultTtlMs: 100, maxTtlMs: 100, identity: { ios: "reusable", android: "reusable" } }, capacity: { strategy: "resource", config: { diff --git a/src/core/nuke.test.ts b/src/core/nuke.test.ts index e326ce1..9a7e669 100644 --- a/src/core/nuke.test.ts +++ b/src/core/nuke.test.ts @@ -169,7 +169,11 @@ function config(): Config { http: { enabled: false, host: "127.0.0.1", port: 4700 }, ios: { slim: { enabled: false, bootTimeoutMs: 600_000 } }, idle: { deleteAfterMs: 10, shutdownAfterMs: 5 }, - lease: { defaultTtlMs: 60_000, maxTtlMs: 3_600_000 }, + lease: { + defaultTtlMs: 60_000, + maxTtlMs: 3_600_000, + identity: { ios: "reusable", android: "reusable" }, + }, capacity: { strategy: "resource", config: { diff --git a/src/core/quarantine-coordinator.test.ts b/src/core/quarantine-coordinator.test.ts index 917e5e1..7e1b7a1 100644 --- a/src/core/quarantine-coordinator.test.ts +++ b/src/core/quarantine-coordinator.test.ts @@ -24,6 +24,8 @@ const retryConfig: QuarantineRetryConfig = { /** In-memory stand-in mirroring the four Registry methods QuarantineCoordinator calls. */ class FakeRegistry implements QuarantineRegistry { #devices: DeviceRecord[]; + /** The initiator of every `deleteQuarantined` commit, in order. */ + readonly deletions: string[] = []; constructor(devices: readonly DeviceRecord[]) { this.#devices = [...devices]; @@ -73,7 +75,8 @@ class FakeRegistry implements QuarantineRegistry { }); } - async abandonQuarantine(deviceId: string): Promise { + async deleteQuarantined(deviceId: string, initiator: string): Promise { + this.deletions.push(initiator); return this.#update(deviceId, (device) => { const { quarantineAttempts: _quarantineAttempts, @@ -109,13 +112,14 @@ async function createHarness( readonly clock?: FakeClock; readonly config?: QuarantineRetryConfig; readonly driver?: FakeDriver; + readonly target?: Pick; } = {}, ) { const clock = options.clock ?? new FakeClock(1_000); const bus = new EventBus(clock); const driver = options.driver ?? new FakeDriver({ clock, platform: "ios" }); const driverDevice = await driver.provision(spec); - const target = device("dev_1", driverDevice.deviceId); + const target = { ...device("dev_1", driverDevice.deviceId), ...options.target }; const registry = new FakeRegistry([target]); const notifyAvailability = vi.fn(); const coordinator = new QuarantineCoordinator({ @@ -257,6 +261,7 @@ describe("QuarantineCoordinator", () => { expect(harness.driver.calls.map((call) => call.operation)).toContain("destroy"); expect(harness.registry.snapshot.devices[0]?.state).toBe("deleted"); + expect(harness.registry.deletions).toEqual(["quarantine-coordinator"]); expect(eventsOf(harness.bus).map((event) => event.event)).toEqual([ "device.purge-failed", "device.quarantined", @@ -307,6 +312,65 @@ describe("QuarantineCoordinator", () => { expect(harness.notifyAvailability).not.toHaveBeenCalled(); }); + it("retries a spent fresh device's delete rather than an erase, and a successful retry reaches deleted, never ready", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ clock, platform: "ios", reclaimResult: "ready" }); + const harness = await createHarness({ + clock, + driver, + target: { lastLeaseEndedAt: 900, leaseIdentity: "fresh" }, + }); + await harness.coordinator.enter({ + attemptedStrategy: "delete", + deviceId: harness.target.id, + duration: 0, + error: "Error: delete exploded", + leaseId: "lease-1", + }); + + harness.clock.advance(retryConfig.retryBackoffMs); + await flush(); + + const operations = harness.driver.calls.map((call) => call.operation); + expect(operations).toContain("destroy"); + expect(operations).not.toContain("reclaim"); + expect(harness.registry.snapshot.devices[0]?.state).toBe("deleted"); + expect(harness.registry.deletions).toEqual(["lease-end"]); + expect(eventsOf(harness.bus).map((event) => event.event)).toEqual([ + "device.purge-failed", + "device.quarantined", + ]); + expect(eventsOf(harness.bus)[0]).toMatchObject({ payload: { attemptedStrategy: "delete" } }); + expect(harness.notifyAvailability).toHaveBeenCalledOnce(); + }); + + it("keeps a spent fresh device quarantined and re-arms when its retried delete fails", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ clock, platform: "ios", reclaimResult: "ready" }); + driver.failOn("destroy", 1, new Error("still stuck")); + const harness = await createHarness({ + clock, + driver, + target: { lastLeaseEndedAt: 900, leaseIdentity: "fresh" }, + }); + await harness.coordinator.enter({ + attemptedStrategy: "delete", + deviceId: harness.target.id, + duration: 0, + error: "boom", + leaseId: "lease-1", + }); + + harness.clock.advance(retryConfig.retryBackoffMs); + await flush(); + + expect(harness.registry.snapshot.devices[0]).toMatchObject({ + quarantineAttempts: 1, + state: "quarantined", + }); + expect(harness.driver.calls.map((call) => call.operation)).not.toContain("reclaim"); + }); + it("never touches a device that left quarantine before its retry timer fires", async () => { const harness = await createHarness(); await harness.coordinator.enter({ diff --git a/src/core/quarantine-coordinator.ts b/src/core/quarantine-coordinator.ts index 2572f5a..3fba849 100644 --- a/src/core/quarantine-coordinator.ts +++ b/src/core/quarantine-coordinator.ts @@ -1,6 +1,6 @@ import type { EventBus } from "../bus/index.js"; import type { Clock, TimerHandle } from "../ports/index.js"; -import type { DeviceRecord, Platform } from "./domain.js"; +import { type DeviceRecord, mayBeGranted, type Platform } from "./domain.js"; import type { Driver, DriverDevice } from "./driver.js"; import type { SerializedDecision } from "./serialized-decision.js"; import { stableError } from "./stable-error.js"; @@ -19,13 +19,14 @@ export interface QuarantineRegistry { ): Promise; recoverFromQuarantine(deviceId: string, to: "ready" | "shutdown"): Promise; strandQuarantine(deviceId: string, attempts: number): Promise; - abandonQuarantine(deviceId: string): Promise; + deleteQuarantined(deviceId: string, initiator: string): Promise; } export interface QuarantinePurgeFailure { readonly deviceId: string; readonly leaseId: string; - readonly attemptedStrategy: "erase" | "snapshot" | "wipe"; + /** A reclaim strategy, or `delete` for a spent fresh device's failed lease-end shutdown or delete. */ + readonly attemptedStrategy: "erase" | "snapshot" | "wipe" | "delete"; readonly duration: number; readonly error: string; } @@ -59,6 +60,8 @@ export interface QuarantineCoordinatorOptions { * * A quarantined device retries its purge on a Clock-driven backoff until it * either succeeds (the device rejoins the warm pool) or exhausts + * `config.maxRetries`. A spent fresh device (see `mayBeGranted`) retries its + * delete instead, never a purge, and success deletes it. Either way, exhausting * `config.maxRetries`, at which point it is destroyed -- never merely shut * down, since `shutdown` is a state AcquisitionPlanner treats as reusable * warm inventory (`boot-shutdown`), which would silently reintroduce the @@ -170,6 +173,10 @@ export class QuarantineCoordinator { const attempts = (device.quarantineAttempts ?? 0) + 1; const driver = this.options.drivers.get(device.spec.platform); + if (!mayBeGranted(device)) { + await this.#retryDelete(driver, device, attempts); + return; + } let result: Awaited>; try { result = await driver.reclaim(toDriverDevice(device), { clean: "standard" }); @@ -189,6 +196,24 @@ export class QuarantineCoordinator { this.options.notifyAvailability(); } + /** + * A spent fresh device already served its one lease, so a successful reclaim would put it + * back in the pool for a second one. It retries the delete instead, and success is + * `device.deleted`, not `device.quarantine-recovered`: nothing rejoined the pool. + */ + async #retryDelete(driver: Driver, device: DeviceRecord, attempts: number): Promise { + try { + await driver.destroy(toDriverDevice(device)); + } catch { + await this.#retryFailed(device, attempts); + return; + } + await this.options.decisions.run(() => + this.options.registry.deleteQuarantined(device.id, "lease-end"), + ); + this.options.notifyAvailability(); + } + async #retryFailed(device: DeviceRecord, attempts: number): Promise { if (attempts >= this.options.config.maxRetries) { await this.#giveUp(device, attempts); @@ -222,7 +247,9 @@ export class QuarantineCoordinator { ); return; } - await this.options.decisions.run(() => this.options.registry.abandonQuarantine(device.id)); + await this.options.decisions.run(() => + this.options.registry.deleteQuarantined(device.id, "quarantine-coordinator"), + ); this.options.eventBus.emit( "device.quarantine-abandoned", { attempts, deviceId: device.id }, diff --git a/src/core/reaper.test.ts b/src/core/reaper.test.ts index 77f306c..7f0d34f 100644 --- a/src/core/reaper.test.ts +++ b/src/core/reaper.test.ts @@ -68,7 +68,7 @@ function config(): Config { http: { enabled: false, host: "127.0.0.1", port: 4700 }, ios: { slim: { enabled: false, bootTimeoutMs: 600_000 } }, idle: { deleteAfterMs: 30_000, shutdownAfterMs: 10_000 }, - lease: { defaultTtlMs: 100, maxTtlMs: 100 }, + lease: { defaultTtlMs: 100, maxTtlMs: 100, identity: { ios: "reusable", android: "reusable" } }, capacity: { strategy: "resource", config: { diff --git a/src/core/registry.test.ts b/src/core/registry.test.ts index 02303e7..31a2e34 100644 --- a/src/core/registry.test.ts +++ b/src/core/registry.test.ts @@ -58,6 +58,7 @@ describe("Registry", () => { driverData: { driverOnly: "value" }, driverDeviceId: "driver_test", id: "dev_test", + leaseIdentity: "reusable", spec, state: "provisioning", }, @@ -120,6 +121,119 @@ describe("Registry", () => { expect(registry.snapshot.devices).toMatchObject([{ id: "dev_legacy", state: "reclaiming" }]); }); + it("loads a registry written before leaseIdentity existed with every device reusable", async () => { + const clock = new FakeClock(1_000); + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/agent/.simlock"); + const legacy = (id: string, platform: "ios" | "android", state: string) => ({ + createdAt: 500, + driverData: {}, + driverDeviceId: `driver_${id}`, + id, + lastLeaseEndedAt: 900, + spec: { ...spec, platform }, + state, + }); + await filesystem.writeFileAtomic( + statePath, + JSON.stringify({ + devices: [legacy("dev_ios", "ios", "shutdown"), legacy("dev_android", "android", "ready")], + leases: [], + }), + ); + + // Loaded under a config that says `fresh`: a record's policy comes from the record, and a + // record with none was created under the only policy there was. + const registry = await Registry.load({ + clock, + eventBus: new EventBus(clock), + filesystem, + idGenerator: { generate: () => "new" }, + leaseIdentity: { android: "fresh", ios: "fresh" }, + statePath, + }); + + expect(registry.snapshot.devices.map((device) => device.leaseIdentity)).toEqual([ + "reusable", + "reusable", + ]); + }); + + it("refuses to load a device record whose leaseIdentity is not a known policy", async () => { + const clock = new FakeClock(1_000); + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/agent/.simlock"); + await filesystem.writeFileAtomic( + statePath, + JSON.stringify({ + devices: [ + { + createdAt: 500, + driverData: {}, + driverDeviceId: "driver_odd", + id: "dev_odd", + leaseIdentity: "disposable", + spec, + state: "ready", + }, + ], + leases: [], + }), + ); + + await expect( + Registry.load({ + clock, + eventBus: new EventBus(clock), + filesystem, + idGenerator: { generate: () => "new" }, + statePath, + }), + ).rejects.toThrow("Invalid device record"); + }); + + it("stamps each registered device with its own platform's policy and keeps it across a reload under another", async () => { + const clock = new FakeClock(1_000); + const filesystem = new MemoryFilesystem(); + let nextId = 1; + const idGenerator = { generate: () => `${nextId++}` }; + const registry = await Registry.load({ + clock, + eventBus: new EventBus(clock), + filesystem, + idGenerator, + leaseIdentity: { android: "reusable", ios: "fresh" }, + statePath, + }); + const ios = await registry.registerDevice({ + driverData: {}, + driverDeviceId: "driver_ios", + provisionDuration: 0, + spec, + }); + const android = await registry.registerDevice({ + driverData: {}, + driverDeviceId: "driver_android", + provisionDuration: 0, + spec: { model: "Pixel 9", osVersion: "36", platform: "android" }, + }); + + expect([ios.leaseIdentity, android.leaseIdentity]).toEqual(["fresh", "reusable"]); + + const reloaded = await Registry.load({ + clock, + eventBus: new EventBus(clock), + filesystem, + idGenerator, + leaseIdentity: { android: "fresh", ios: "reusable" }, + statePath, + }); + expect(reloaded.snapshot.devices.map((device) => device.leaseIdentity)).toEqual([ + "fresh", + "reusable", + ]); + }); + it("preserves unknown persisted fields when saving a later mutation", async () => { const clock = new FakeClock(1_000); const filesystem = new MemoryFilesystem(); @@ -332,6 +446,41 @@ describe("Registry", () => { }); }); + it("enters quarantine from shutdown, a spent fresh device's failed-delete entry point", async () => { + const clock = new FakeClock(1_000); + const registry = await Registry.load({ + clock, + eventBus: new EventBus(clock), + filesystem: new MemoryFilesystem(), + idGenerator: { generate: () => "test" }, + leaseIdentity: { android: "reusable", ios: "fresh" }, + statePath, + }); + const device = await registry.registerDevice({ + driverData: {}, + driverDeviceId: "driver_test", + provisionDuration: 0, + spec, + }); + await registry.transitionDevice(device.id, "ready", { + event: "device.ready", + payload: { bootDuration: 0, deviceId: device.id }, + }); + const lease = await registry.createLease({ + deviceId: device.id, + requesterId: "agent-1", + ownerId: "agent-1", + ttlMs: 60_000, + ttlDeadline: 2_000, + }); + await registry.beginRelease(lease.id); + await registry.completeReclaimWithoutPurge(device.id); + + const quarantined = await registry.enterQuarantine(device.id, 5_000); + + expect(quarantined).toMatchObject({ quarantineNextRetryAt: 5_000, state: "quarantined" }); + }); + it("refuses to quarantine a device that is still leased", async () => { // Quarantine is a post-release disposition: it is only ever entered from `reclaiming`, which // a device reaches by having its lease released. A leased device reaching it would mean @@ -444,6 +593,7 @@ describe("Registry", () => { driverDeviceId: "driver_test", id: device.id, lastLeaseEndedAt: 1_000, + leaseIdentity: "reusable", spec, state: "ready", }); @@ -452,11 +602,11 @@ describe("Registry", () => { ); }); - it("abandons a quarantined device to deleted and emits device.deleted", async () => { + it("deletes a quarantined device and emits device.deleted with the caller's initiator", async () => { const clock = new FakeClock(1_000); const bus = new EventBus(clock); - const events: string[] = []; - bus.subscribe("device.deleted", (envelope) => events.push(envelope.event)); + const events: unknown[] = []; + bus.subscribe("device.deleted", (envelope) => events.push(envelope.payload)); const registry = await Registry.load({ clock, eventBus: bus, @@ -484,10 +634,10 @@ describe("Registry", () => { await registry.beginRelease(lease.id); await registry.enterQuarantine(device.id, 5_000); - const abandoned = await registry.abandonQuarantine(device.id); + const deleted = await registry.deleteQuarantined(device.id, "lease-end"); - expect(abandoned.state).toBe("deleted"); - expect(events).toEqual(["device.deleted"]); + expect(deleted.state).toBe("deleted"); + expect(events).toEqual([{ deviceId: device.id, initiator: "lease-end" }]); }); it("rejects mutations for a device that is not registered", async () => { diff --git a/src/core/registry.ts b/src/core/registry.ts index 0988a6b..f0209c0 100644 --- a/src/core/registry.ts +++ b/src/core/registry.ts @@ -6,6 +6,7 @@ import { type DeviceSpec, type DeviceState, type DeviceTransitionUpdate, + type LeaseIdentity, type LeaseRecord, type Platform, transition, @@ -26,6 +27,12 @@ export interface RegistryOptions { * a granted lease's width always comes from the grant itself. */ readonly defaultTtlMs?: number; + /** + * `lease.identity`: the policy a newly registered device is stamped with, looked up by its + * spec's platform. Only registration reads it -- a loaded record keeps the policy it was + * created under. Defaults to `reusable` for both platforms. + */ + readonly leaseIdentity?: Readonly>; } export interface RegistrySnapshot { @@ -101,6 +108,7 @@ export class Registry { const registry = new Registry({ ...options, defaultTtlMs: options.defaultTtlMs ?? DEFAULT_LEASE_TTL_MS, + leaseIdentity: options.leaseIdentity ?? { android: "reusable", ios: "reusable" }, statePath: options.statePath ?? DEFAULT_REGISTRY_PATH, }); @@ -129,6 +137,7 @@ export class Registry { driverData, driverDeviceId, id: `dev_${this.options.idGenerator.generate()}`, + leaseIdentity: this.options.leaseIdentity[spec.platform], spec: { ...spec }, state: "provisioning", }; @@ -183,9 +192,14 @@ export class Registry { return cloneDevice(updated); } - /** Commits an unleased reclaim interrupted by daemon shutdown, found still `reclaiming` at startup. */ + /** + * Commits `reclaiming -> shutdown` for a device whose driver shutdown ran without a purge, so + * no `device.reclaimed` fact fits: an unleased reclaim interrupted by daemon shutdown and found + * at startup, or a spent fresh device's lease-end shutdown before its delete. The caller emits + * whatever fact its own path owns. + */ // fallow-ignore-next-line unused-class-member -- called through WarmPoolCoordinator's registry port. - async completeInterruptedReclaim(deviceId: string): Promise { + async completeReclaimWithoutPurge(deviceId: string): Promise { const { device, index } = this.#requireDeviceRecord(deviceId); if (device.state !== "reclaiming") { throw new RegistryEventError(`Device is not reclaiming: ${deviceId}`); @@ -198,18 +212,24 @@ export class Registry { } /** - * Commits quarantine entry from either of its two legal sources (see domain.ts): - * a release-time purge failure leaves `reclaiming`, and a stalled-transition - * timeout leaves `provisioning`. The caller -- WarmPoolCoordinator for the former, - * QuarantineCoordinator's stalled-transition entry point for the latter -- emits - * `device.quarantined` (and, for a purge failure, `device.purge-failed`) after this - * commits. + * Commits quarantine entry from any of its legal sources (see domain.ts): a + * release-time purge failure, or a fresh device's failed lease-end shutdown, leaves + * `reclaiming`; a fresh device's failed delete leaves `shutdown`; and a + * stalled-transition timeout leaves `provisioning`. The caller -- QuarantineCoordinator, + * reached from WarmPoolCoordinator for the lease-end paths -- emits `device.quarantined` + * (and, for a lease-end failure, `device.purge-failed`) after this commits. */ // fallow-ignore-next-line unused-class-member -- called through QuarantineCoordinator's registry port. async enterQuarantine(deviceId: string, nextRetryAt: number): Promise { const { device, index } = this.#requireDeviceRecord(deviceId); - if (device.state !== "reclaiming" && device.state !== "provisioning") { - throw new RegistryEventError(`Device is not reclaiming or provisioning: ${deviceId}`); + if ( + device.state !== "reclaiming" && + device.state !== "provisioning" && + device.state !== "shutdown" + ) { + throw new RegistryEventError( + `Device is not reclaiming, provisioning, or shutdown: ${deviceId}`, + ); } const updated: DeviceRecord = { ...transition(device, "quarantined"), @@ -264,7 +284,6 @@ export class Registry { return cloneDevice(updated as DeviceRecord); } - /** Commits giving up on a quarantined device once its driver `destroy` has already run. */ /** * Records that a quarantined device exhausted its retries *and* could not be destroyed, so * nothing is armed for it any more. Clears the retry deadline rather than leaving a stale one @@ -286,8 +305,13 @@ export class Registry { return cloneDevice(updated); } + /** + * Commits `quarantined -> deleted` once the driver `destroy` has already run: a spent fresh + * device whose retried delete succeeded (initiator `lease-end`), or any device quarantine gave + * up on (initiator `quarantine-coordinator`). + */ // fallow-ignore-next-line unused-class-member -- called through QuarantineCoordinator's registry port. - async abandonQuarantine(deviceId: string): Promise { + async deleteQuarantined(deviceId: string, initiator: string): Promise { const { device, index } = this.#requireDeviceRecord(deviceId); if (device.state !== "quarantined") { throw new RegistryEventError(`Device is not quarantined: ${deviceId}`); @@ -301,11 +325,7 @@ export class Registry { const devices = [...this.#devices]; devices[index] = updated as DeviceRecord; await this.#commit(devices, this.#leases); - this.options.eventBus.emit( - "device.deleted", - { deviceId, initiator: "quarantine-coordinator" }, - "registry", - ); + this.options.eventBus.emit("device.deleted", { deviceId, initiator }, "registry"); return cloneDevice(updated as DeviceRecord); } @@ -586,6 +606,7 @@ const deviceRecordKeys = [ "quarantineNextRetryAt", "address", "featureProfile", + "leaseIdentity", ] as const; const leaseRecordKeys = [ "id", @@ -698,11 +719,23 @@ function parseDevice(value: unknown): DeviceRecord { driverData, driverDeviceId, id, + leaseIdentity: parseLeaseIdentity(value.leaseIdentity), spec, state: state === "warm" ? "reclaiming" : state, }; } +/** + * A record written before `leaseIdentity` existed was created under the only policy there was, + * so it loads as `reusable`. Unlike `featureProfile`, a present-but-unknown value fails the + * load: guessing `reusable` for it could hand a fresh identity out for a second lease. + */ +function parseLeaseIdentity(value: unknown): LeaseIdentity { + if (value === undefined) return "reusable"; + if (value === "reusable" || value === "fresh") return value; + throw new RegistryLoadError("Invalid device record in registry state"); +} + /** * Unlike `address`, a garbage `featureProfile` is dropped rather than failing the whole record -- * it is a derived, re-derivable-on-next-boot hint (see `domain.ts`), not load-bearing identity, so diff --git a/src/core/startup-converger.test.ts b/src/core/startup-converger.test.ts index caf4c2f..3c01401 100644 --- a/src/core/startup-converger.test.ts +++ b/src/core/startup-converger.test.ts @@ -79,6 +79,12 @@ function createHarness( }), }; const quarantineRestore = { restore: vi.fn(() => void order.push("quarantine-restore")) }; + const spentDeviceDeletion = { + deleteSpent: vi.fn(async (target: DeviceRecord) => { + order.push(`delete-spent:${target.id}`); + updateState(target.id, "deleted"); + }), + }; const converger = new StartupConverger({ capacity: { deviceLimit: () => limits.ios + limits.android, @@ -97,6 +103,7 @@ function createHarness( return { devices, leases }; }, }, + spentDeviceDeletion, timers, }); @@ -119,6 +126,7 @@ function createHarness( order, quarantineRestore, recovery, + spentDeviceDeletion, timers, }; } @@ -308,6 +316,34 @@ describe("StartupConverger", () => { expect(harness.devices.find((item) => item.id === leasedDevice.id)?.state).toBe("leased"); }); + it("deletes every spent fresh device found reclaiming or shutdown, after recovering interrupted reclaims", async () => { + // `device()` stamps `lastLeaseEndedAt` on every record, so the reusable one below has ended + // a lease too: only its identity policy keeps it out of the delete. + const spentReclaiming = { ...device("spent-reclaiming", "ios", "reclaiming", 1) }; + const spentShutdown = { ...device("spent-shutdown", "ios", "shutdown", 2) }; + const reusableShutdown = device("reusable-shutdown", "ios", "shutdown", 3); + const harness = createHarness( + [ + { ...spentReclaiming, leaseIdentity: "fresh" }, + { ...spentShutdown, leaseIdentity: "fresh" }, + reusableShutdown, + ], + [], + { android: 3, global: 3, ios: 3 }, + ); + + await harness.converger.converge(); + + expect(harness.order).toEqual([ + "timers", + "quarantine-restore", + "recover:spent-reclaiming", + "delete-spent:spent-reclaiming", + "delete-spent:spent-shutdown", + ]); + expect(harness.devices.find((item) => item.id === reusableShutdown.id)?.state).toBe("shutdown"); + }); + it("restores timers before quarantine and interrupted-reclaim recovery", async () => { const reclaiming = device("reclaiming", "ios", "reclaiming", 1); const harness = createHarness([reclaiming], [], { android: 1, global: 3, ios: 3 }); diff --git a/src/core/startup-converger.ts b/src/core/startup-converger.ts index 9010f5d..3120faf 100644 --- a/src/core/startup-converger.ts +++ b/src/core/startup-converger.ts @@ -1,5 +1,5 @@ import type { CleanupActionExecutor } from "./cleanup-executor.js"; -import type { DeviceRecord, LeaseRecord, Platform } from "./domain.js"; +import { type DeviceRecord, type LeaseRecord, mayBeGranted, type Platform } from "./domain.js"; import type { CapacityReader } from "./lease-ports.js"; import type { SerializedDecision } from "./serialized-decision.js"; import { compareLeastRecentlyUsed } from "./warm-pool.js"; @@ -21,6 +21,11 @@ export interface InterruptedReclaimRecovery { recoverInterruptedReclaim(device: DeviceRecord): Promise; } +/** Deletes a spent fresh device the previous process shut down but did not delete. */ +export interface SpentDeviceDeletion { + deleteSpent(device: DeviceRecord): Promise; +} + /** Re-arms retry timers for devices still `quarantined` at startup, from persisted state. */ export interface QuarantineRestorer { restore(): void; @@ -45,6 +50,7 @@ export interface StartupConvergerOptions { readonly interruptedReclaimRecovery: InterruptedReclaimRecovery; readonly quarantineRestore: QuarantineRestorer; readonly registry: StartupRegistry; + readonly spentDeviceDeletion: SpentDeviceDeletion; readonly timers: LeaseTimerRestorer; } @@ -82,6 +88,9 @@ export class StartupConverger { // either step. this.options.quarantineRestore.restore(); await this.#recoverInterruptedReclaims(); + // After interrupted reclaims: a spent fresh device found `reclaiming` has just been shut + // down there, and is deleted here along with any the previous process left `shutdown`. + await this.#deleteSpentDevices(); const refused = new Set(); for (;;) { @@ -115,6 +124,24 @@ export class StartupConverger { } } + async #deleteSpentDevices(): Promise { + const spent = await this.options.decisions.run(() => { + const snapshot = this.options.registry.snapshot; + const leasedDeviceIds = new Set(snapshot.leases.map((lease) => lease.deviceId)); + return snapshot.devices.filter( + (device) => + device.state === "shutdown" && + !mayBeGranted(device) && + this.options.drivers.has(device.spec.platform) && + !leasedDeviceIds.has(device.id) && + !this.options.claims.isClaimed(device.id), + ); + }); + for (const device of spent) { + await this.options.spentDeviceDeletion.deleteSpent(device); + } + } + #nextExcessCandidate(refused: ReadonlySet): DeviceRecord | undefined { const capacity = this.options.capacity.runningCapacity; const overPlatforms = (["ios", "android"] as const).filter( diff --git a/src/core/warm-pool-coordinator.test.ts b/src/core/warm-pool-coordinator.test.ts index c5f34f2..170444f 100644 --- a/src/core/warm-pool-coordinator.test.ts +++ b/src/core/warm-pool-coordinator.test.ts @@ -10,7 +10,11 @@ import { DriverCatalog } from "./driver-catalog.js"; import type { QuarantinePurgeFailure } from "./quarantine-coordinator.js"; import type { ReleasedLease } from "./registry.js"; import { SerializedDecision } from "./serialized-decision.js"; -import { WarmPoolCoordinator, type WarmPoolQuarantine } from "./warm-pool-coordinator.js"; +import { + WarmPoolCoordinator, + type WarmPoolQuarantine, + type WarmPoolRegistry, +} from "./warm-pool-coordinator.js"; const gibibyte = 1024 ** 3; const spec = { model: "iPhone 16", osVersion: "26.5", platform: "ios" } as const; @@ -47,7 +51,7 @@ const config: Config = { retryBackoffMultiplier: 2, }, }, - lease: { defaultTtlMs: 100, maxTtlMs: 100 }, + lease: { defaultTtlMs: 100, maxTtlMs: 100, identity: { ios: "reusable", android: "reusable" } }, capacity: { strategy: "resource", config: { @@ -83,15 +87,8 @@ class TestRegistry { async transitionDevice( deviceId: string, - to: "ready" | "shutdown", - event: { - readonly event: "device.reclaimed"; - readonly payload: { - readonly deviceId: string; - readonly duration: number; - readonly strategy: "erase" | "snapshot" | "wipe"; - }; - }, + to: Parameters[1], + event: Parameters[2], update?: DeviceTransitionUpdate, ): Promise { const index = this.#devices.findIndex((device) => device.id === deviceId); @@ -100,11 +97,15 @@ class TestRegistry { this.lastUpdate = update; const updated = { ...current, ...update, state: to } as DeviceRecord; this.#devices[index] = updated; - this.eventBus.emit("device.reclaimed", event.payload, "test-registry"); + if (event.event === "device.reclaimed") { + this.eventBus.emit(event.event, event.payload, "test-registry"); + } else { + this.eventBus.emit(event.event, event.payload, "test-registry"); + } return updated; } - async completeInterruptedReclaim(deviceId: string): Promise { + async completeReclaimWithoutPurge(deviceId: string): Promise { const index = this.#devices.findIndex((device) => device.id === deviceId); const current = this.#devices[index]; if (index === -1 || current === undefined) throw new Error("missing device"); @@ -326,4 +327,96 @@ describe("WarmPoolCoordinator", () => { expect(harness.driver.calls.map((call) => call.operation)).toContain("shutdown"); expect(harness.notifyAvailability).toHaveBeenCalledOnce(); }); + + describe("a spent fresh device", () => { + function spent(record: DeviceRecord): DeviceRecord { + return { ...record, lastLeaseEndedAt: 900, leaseIdentity: "fresh" }; + } + + async function freshHarness(state: DeviceRecord["state"]) { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ clock, platform: "ios", reclaimResult: "ready" }); + const target = spent(device("fresh", state, (await driver.provision(spec)).deviceId, spec)); + const harness = await createHarness({ devices: [target], driver }); + const operations = () => + driver.calls.map((call) => call.operation).filter((operation) => operation !== "provision"); + return { ...harness, operations, target }; + } + + it("is never erased: its lease end is a shutdown and a delete", async () => { + const harness = await freshHarness("reclaiming"); + + await harness.coordinator.reclaim(released(harness.target)); + + expect(harness.operations()).toEqual(["shutdown", "destroy"]); + expect(harness.registry.snapshot.devices[0]?.state).toBe("deleted"); + expect( + harness.bus.replay().map((event) => ({ event: event.event, payload: event.payload })), + ).toEqual([ + { + event: "device.deleted", + payload: { deviceId: harness.target.id, initiator: "lease-end" }, + }, + ]); + expect(harness.quarantined).toEqual([]); + }); + + it("hands a failed delete to quarantine with strategy delete, after the shutdown commit", async () => { + const harness = await freshHarness("reclaiming"); + harness.driver.failOn("destroy", 1, new Error("delete exploded")); + + await harness.coordinator.reclaim(released(harness.target)); + + expect(harness.registry.snapshot.devices[0]?.state).toBe("shutdown"); + expect(harness.quarantined).toEqual([ + { + attemptedStrategy: "delete", + deviceId: harness.target.id, + duration: 0, + error: "Error: delete exploded", + leaseId: "lease-1", + }, + ]); + expect(harness.bus.replay().map((event) => event.event)).not.toContain("device.deleted"); + }); + + it("hands a failed lease-end shutdown to quarantine with strategy delete, without deleting", async () => { + const harness = await freshHarness("reclaiming"); + harness.driver.failOn("shutdown", 1, new Error("shutdown exploded")); + + await harness.coordinator.reclaim(released(harness.target)); + + expect(harness.registry.snapshot.devices[0]?.state).toBe("reclaiming"); + expect(harness.operations()).toEqual(["shutdown"]); + expect(harness.quarantined).toMatchObject([ + { attemptedStrategy: "delete", error: "Error: shutdown exploded" }, + ]); + }); + + it("is deleted by deleteSpent when found shutdown, while a reusable shutdown device is left alone", async () => { + const harness = await freshHarness("shutdown"); + const reusable = device("reusable", "shutdown", "reusable-driver", spec); + const registry = new TestRegistry([harness.target, reusable], [], harness.bus); + const coordinator = new WarmPoolCoordinator({ + capacity: capacity(), + clock: harness.clock, + decisions: new SerializedDecision(), + drivers: new DriverCatalog([harness.driver]), + eventBus: harness.bus, + notifyAvailability: harness.notifyAvailability, + quarantine: harness.quarantine, + queueHeadDemand: () => undefined, + registry, + }); + + await expect(coordinator.deleteSpent(reusable.id)).resolves.toBe(false); + await expect(coordinator.deleteSpent(harness.target.id)).resolves.toBe(true); + + expect(harness.operations()).toEqual(["destroy"]); + expect(registry.snapshot.devices.map((record) => record.state)).toEqual([ + "deleted", + "shutdown", + ]); + }); + }); }); diff --git a/src/core/warm-pool-coordinator.ts b/src/core/warm-pool-coordinator.ts index 01735e2..8875155 100644 --- a/src/core/warm-pool-coordinator.ts +++ b/src/core/warm-pool-coordinator.ts @@ -6,6 +6,7 @@ import { type DeviceSpec, type DeviceTransitionUpdate, type LeaseRecord, + mayBeGranted, type Platform, sameSpec, } from "./domain.js"; @@ -26,18 +27,23 @@ export interface WarmPoolRegistry { }; transitionDevice( deviceId: string, - to: "ready" | "shutdown", - event: { - readonly event: "device.reclaimed"; - readonly payload: { - readonly deviceId: string; - readonly duration: number; - readonly strategy: "erase" | "snapshot" | "wipe"; - }; - }, + to: "ready" | "shutdown" | "deleted", + event: + | { + readonly event: "device.reclaimed"; + readonly payload: { + readonly deviceId: string; + readonly duration: number; + readonly strategy: "erase" | "snapshot" | "wipe"; + }; + } + | { + readonly event: "device.deleted"; + readonly payload: { readonly deviceId: string; readonly initiator: string }; + }, update?: DeviceTransitionUpdate, ): Promise; - completeInterruptedReclaim(deviceId: string): Promise; + completeReclaimWithoutPurge(deviceId: string): Promise; } export interface WarmPoolCapacityReader { @@ -70,6 +76,10 @@ export class WarmPoolCoordinator { constructor(private readonly options: WarmPoolCoordinatorOptions) {} async reclaim(released: ReleasedLease): Promise { + if (!mayBeGranted(released.device)) { + await this.#retireSpent(released); + return; + } const driver = this.options.drivers.get(released.device.spec.platform); const startedAt = this.options.clock.now(); const attemptedStrategy = driver.reclaimStrategy({ clean: "standard" }); @@ -127,7 +137,7 @@ export class WarmPoolCoordinator { (lease) => lease.deviceId === deviceId, ); if (current?.state !== "reclaiming" || leased) return false; - await this.options.registry.completeInterruptedReclaim(deviceId); + await this.options.registry.completeReclaimWithoutPurge(deviceId); this.options.eventBus.emit( "device.shutdown", { deviceId, initiator: "startup-interrupted-reclaim" }, @@ -139,6 +149,72 @@ export class WarmPoolCoordinator { return recovered; } + /** + * Finishes a spent fresh device found `shutdown` at startup: the daemon stopped after its + * lease-end shutdown committed and before its delete did. A delete that fails throws and + * leaves the device `shutdown`, where `mayBeGranted` still keeps it out of every grant. + */ + async deleteSpent(deviceId: string): Promise { + const device = await this.options.decisions.run(async () => { + const current = this.#unleasedDevice(deviceId, "shutdown"); + return current === undefined || mayBeGranted(current) ? undefined : current; + }); + if (device === undefined) return false; + return this.#deleteSpent(device); + } + + /** + * A spent fresh device is never purged and never returns to the pool. Its lease end is a + * driver shutdown, a `reclaiming -> shutdown` commit with no `device.reclaimed` (nothing was + * reclaimed), then the delete. Either driver step failing hands the device to quarantine, + * which retries the delete. + */ + async #retireSpent(released: ReleasedLease): Promise { + const driver = this.options.drivers.get(released.device.spec.platform); + const startedAt = this.options.clock.now(); + try { + await driver.shutdown(toDriverDevice(released.device)); + } catch (error: unknown) { + await this.#recoverPurgeFailure(released, startedAt, "delete", error); + return; + } + const shutdown = await this.options.decisions.run(() => + this.options.registry.completeReclaimWithoutPurge(released.device.id), + ); + // Running capacity is free from here; the device itself stays ungrantable. + this.options.notifyAvailability(); + + try { + await this.#deleteSpent(shutdown); + } catch (error: unknown) { + await this.#recoverPurgeFailure(released, startedAt, "delete", error); + } + } + + /** Destroys a spent device and commits `shutdown -> deleted`, if it is still that device. */ + async #deleteSpent(device: DeviceRecord): Promise { + await this.options.drivers.get(device.spec.platform).destroy(toDriverDevice(device)); + const deleted = await this.options.decisions.run(async () => { + if (this.#unleasedDevice(device.id, "shutdown") === undefined) return false; + await this.options.registry.transitionDevice(device.id, "deleted", { + event: "device.deleted", + payload: { deviceId: device.id, initiator: "lease-end" }, + }); + return true; + }); + if (deleted) this.options.notifyAvailability(); + return deleted; + } + + #unleasedDevice(deviceId: string, state: DeviceRecord["state"]): DeviceRecord | undefined { + const { devices, leases } = this.options.registry.snapshot; + const current = devices.find((candidate) => candidate.id === deviceId); + if (current?.state !== state || leases.some((lease) => lease.deviceId === deviceId)) { + return undefined; + } + return current; + } + /** * Hands the release-time purge failure to the quarantine coordinator instead * of readiness-checking the device back into circulation: the first warm-pool @@ -148,7 +224,7 @@ export class WarmPoolCoordinator { async #recoverPurgeFailure( released: ReleasedLease, startedAt: number, - attemptedStrategy: "erase" | "snapshot" | "wipe", + attemptedStrategy: QuarantinePurgeFailure["attemptedStrategy"], error: unknown, ): Promise { await this.options.quarantine.enter({ diff --git a/src/daemon/dispatcher.test.ts b/src/daemon/dispatcher.test.ts index 8888586..749f767 100644 --- a/src/daemon/dispatcher.test.ts +++ b/src/daemon/dispatcher.test.ts @@ -1334,7 +1334,12 @@ function testConfig( http: { enabled: false, host: "127.0.0.1", port: 4700 }, ios: { slim: { enabled: false, bootTimeoutMs: 600_000 } }, idle: { deleteAfterMs: 60_000, shutdownAfterMs: 10_000 }, - lease: { defaultTtlMs: 60_000, maxTtlMs: 3_600_000, ...leaseOverrides }, + lease: { + defaultTtlMs: 60_000, + maxTtlMs: 3_600_000, + identity: { ios: "reusable", android: "reusable" }, + ...leaseOverrides, + }, capacity: { strategy: "resource", config: { diff --git a/src/daemon/main.ts b/src/daemon/main.ts index 23713a7..078cb8f 100644 --- a/src/daemon/main.ts +++ b/src/daemon/main.ts @@ -159,6 +159,7 @@ export async function startDaemon(options: StartDaemonOptions = {}): Promise Date: Mon, 14 Sep 2026 13:16:45 +0200 Subject: [PATCH 2/2] fix(core): keep a spent fresh device shutdown under doctor --fix doctor --fix moved a shutdown device observed running to ready. For a spent fresh device that stranded it: every delete path selects shutdown, so it sat ungrantable in the warm pool until the idle rules removed it. The fix now skips spent devices. Also folds startup convergence's three copies of the actionable-device filter into one helper. Refs #75 --- src/core/doctor.test.ts | 58 +++++++++++++++++++++++++++++++++++ src/core/doctor.ts | 10 +++++- src/core/startup-converger.ts | 53 ++++++++++++++------------------ 3 files changed, 90 insertions(+), 31 deletions(-) diff --git a/src/core/doctor.test.ts b/src/core/doctor.test.ts index 3cb32f4..ea7666a 100644 --- a/src/core/doctor.test.ts +++ b/src/core/doctor.test.ts @@ -1159,6 +1159,64 @@ describe("Doctor", () => { expect(shutdown?.foreignStateDetectedAt).toBeUndefined(); }); + it("--fix keeps a spent fresh device shutdown when it is observed running, so its delete still finds it", async () => { + const clock = new FakeClock(10_000); + const eventBus = new EventBus(clock); + const registry = await Registry.load({ + clock, + eventBus, + filesystem: new MemoryFilesystem(), + idGenerator: sequence(), + leaseIdentity: { android: "reusable", ios: "fresh" }, + statePath: "/state.json", + }); + const spent = await registry.registerDevice({ + driverData: {}, + driverDeviceId: "simlock-spent", + provisionDuration: 0, + spec: { model: "iPhone 16", osVersion: "26.5", platform: "ios" }, + }); + await registry.transitionDevice(spent.id, "ready", { + event: "device.ready", + payload: { bootDuration: 0, deviceId: spent.id }, + }); + const lease = await registry.createLease({ + deviceId: spent.id, + ownerId: "gone", + requesterId: "gone", + ttlDeadline: 60_000, + ttlMs: 60_000, + }); + await registry.beginRelease(lease.id); + await registry.completeReclaimWithoutPurge(spent.id); + + const driver = new FakeDriver({ clock, platform: "ios" }); + driver.setManagedReality({ + devices: [ + { + address: "spent-address", + deviceId: "simlock-spent", + driverData: {}, + runState: "running", + }, + ], + processes: [], + }); + + const report = await new Doctor({ + clock, + config: config(), + drivers: [driver], + eventBus, + registry, + }).reconcile({ fix: true }); + + expect(report.findings.map((finding) => finding.kind)).toContain("foreign-state-change"); + expect(registry.snapshot.devices.find((device) => device.id === spent.id)?.state).toBe( + "shutdown", + ); + }); + it("--fix leaves a leased device untouched while still reporting the finding", async () => { const clock = new FakeClock(10_000); const eventBus = new EventBus(clock); diff --git a/src/core/doctor.ts b/src/core/doctor.ts index 9375c51..effbec1 100644 --- a/src/core/doctor.ts +++ b/src/core/doctor.ts @@ -5,6 +5,7 @@ import { type DeviceRecord, type DeviceSpec, type DeviceState, + mayBeGranted, type Platform, transitionEnteredAt, } from "./domain.js"; @@ -578,7 +579,14 @@ export class Doctor { event: "device.shutdown", payload: { deviceId: finding.deviceId, initiator: "doctor" }, }); - } else if (device.state === "shutdown" && finding.observed === "running") { + } else if ( + device.state === "shutdown" && + finding.observed === "running" && + // A spent fresh device stays `shutdown`, running or not: every path that deletes it + // selects `shutdown`, and the delete stops a running simulator first. Moving it to + // `ready` would strand it as ungrantable warm inventory that nothing deletes. + mayBeGranted(device) + ) { await this.options.registry.transitionDevice(finding.deviceId, "ready", { event: "device.ready", payload: { bootDuration: 0, deviceId: finding.deviceId }, diff --git a/src/core/startup-converger.ts b/src/core/startup-converger.ts index 3120faf..e201844 100644 --- a/src/core/startup-converger.ts +++ b/src/core/startup-converger.ts @@ -108,35 +108,18 @@ export class StartupConverger { } async #recoverInterruptedReclaims(): Promise { - const interrupted = await this.options.decisions.run(() => { - const snapshot = this.options.registry.snapshot; - const leasedDeviceIds = new Set(snapshot.leases.map((lease) => lease.deviceId)); - return snapshot.devices.filter( - (device) => - device.state === "reclaiming" && - this.options.drivers.has(device.spec.platform) && - !leasedDeviceIds.has(device.id) && - !this.options.claims.isClaimed(device.id), - ); - }); + const interrupted = await this.options.decisions.run(() => + this.#actionableDevices("reclaiming"), + ); for (const device of interrupted) { await this.options.interruptedReclaimRecovery.recoverInterruptedReclaim(device); } } async #deleteSpentDevices(): Promise { - const spent = await this.options.decisions.run(() => { - const snapshot = this.options.registry.snapshot; - const leasedDeviceIds = new Set(snapshot.leases.map((lease) => lease.deviceId)); - return snapshot.devices.filter( - (device) => - device.state === "shutdown" && - !mayBeGranted(device) && - this.options.drivers.has(device.spec.platform) && - !leasedDeviceIds.has(device.id) && - !this.options.claims.isClaimed(device.id), - ); - }); + const spent = await this.options.decisions.run(() => + this.#actionableDevices("shutdown").filter((device) => !mayBeGranted(device)), + ); for (const device of spent) { await this.options.spentDeviceDeletion.deleteSpent(device); } @@ -151,18 +134,28 @@ export class StartupConverger { return undefined; } - const snapshot = this.options.registry.snapshot; - const leasedDeviceIds = new Set(snapshot.leases.map((lease) => lease.deviceId)); - return snapshot.devices + return this.#actionableDevices("ready") .filter( (device) => - device.state === "ready" && - this.options.drivers.has(device.spec.platform) && - !leasedDeviceIds.has(device.id) && - !this.options.claims.isClaimed(device.id) && !refused.has(device.id) && (overPlatforms.length === 0 || overPlatforms.includes(device.spec.platform)), ) .sort(compareLeastRecentlyUsed)[0]; } + + /** + * Devices in `state` that startup may act on: their platform has a driver, no lease holds + * them, and no in-process operation has claimed them. Read inside a decision section. + */ + #actionableDevices(state: DeviceRecord["state"]): DeviceRecord[] { + const snapshot = this.options.registry.snapshot; + const leasedDeviceIds = new Set(snapshot.leases.map((lease) => lease.deviceId)); + return snapshot.devices.filter( + (device) => + device.state === state && + this.options.drivers.has(device.spec.platform) && + !leasedDeviceIds.has(device.id) && + !this.options.claims.isClaimed(device.id), + ); + } }