diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 14831ce..aa36b0d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -458,6 +458,55 @@ Events are emitted post-commit only; handler failures are isolated from emitters. See [EVENTS.md](EVENTS.md) and [agent-rules/events.md](agent-rules/events.md). +### Driver facts reach the bus through diagnostics, never directly + +Drivers must never depend on the event bus (architecture rule 5 — a driver is +not an observer of its own facts). Where a driver needs to report something +the daemon should turn into a bus event, it reports it through its own +`onDiagnostic` callback option instead — the Android driver already did this +for `snapshot-cold-boot` and unreadable device-profile sources; the iOS +driver gained the same option for component installs. `src/daemon/main.ts` +wires each driver's `onDiagnostic` at construction time +(`discoverDrivers`), bridging the diagnostic to `component.install-started` / +`component.installed` / `component.install-failed` — see +[EVENTS.md](EVENTS.md#components). This is also why those events are +attributed to the `driver-diagnostics` emitter rather than to `IosSimctlDriver` +or `AndroidDriver` directly: the driver only observed the fact, the daemon +layer is what committed it to the bus. Both drivers also thread the +requesting lease's `requesterId` (when `resolveSpec`'s caller knew one — see +`LeaseAcquisitionCoordinator#resolveAndDrive`) through the diagnostic into +the bridged event's payload, so a component install is attributable to the +request that caused it. + +`component.installed` is a verified fact, not "the installer exited 0": +`xcodebuild`/`sdkmanager` reporting success only means the tool claims to +have finished, not that the thing the request actually needed — a runtime at +the requested version, paired with the requested device type for iOS; the +requested system image for Android — is now present. Both drivers re-scan +their own catalog (`simctl list` / the SDK's `system-images` tree) after the +installer returns and only report `component.installed` once that re-scan +confirms it; a re-scan that comes up empty reports `component.install-failed` +with that reason instead, and the caller still sees the same typed error it +always did (`DriverCrashError` for iOS's "still not installed" case, +`IosRuntimeUnpairedError` for a downloaded-but-unpaired runtime). Exactly one +terminal fact fires per install attempt, matching the pre-existing +`component.install-started` timing. + +Before starting either install, the driver reserves free disk space against a +conservative per-component estimate (~8 GiB for an iOS runtime, ~2 GiB for an +Android system image) through a `DiskSpaceGuard` shared across every driver +(`src/daemon/main.ts` constructs one instance and passes it to each driver's +options) rather than a bare instantaneous `Filesystem#diskFree` reading: two +concurrent installs — an iOS runtime download racing an Android system-image +install, or two of either — could otherwise each observe enough free space +individually and jointly overfill the volume neither alone would have. The +guard tracks bytes reserved but not yet released, keyed per path, and checks +free space *minus* those outstanding reservations; the reservation is +released once the install settles either way. A reservation that doesn't fit +still fails fast with the same typed `InsufficientDiskSpaceError` naming +required vs. available bytes, and no `component.install-*` diagnostic fires +for a preflight failure, since no install was actually attempted. + ## External APIs behind interfaces (ports) Every external API the app touches gets its own type/interface (a *port*), @@ -561,10 +610,44 @@ cannot depend on `config.log` having loaded successfully, so it builds its own logger straight from the default log path at a fixed level, falling back to `console.error` only if that itself fails. +`startDaemon` also subscribes `logger.child("components")` to `component.installed` +(`wireComponentInstallLogging` in `src/daemon/main.ts`) so a component simlock +installed on an agent's behalf stays attributable in `daemon.log` after the +event ring buffer resets on restart — the same durable-vs-ring-buffer split as +everything else in this section, applied to component installs specifically +because there is no registry entry or uninstall for them to be recovered from +otherwise (see "Out of scope" in the #67 issue). The log line carries +`requesterId` whenever the event payload has one, so the durable record names +which agent's request caused the install, not just that one happened. + ## Device requests Required to identify a device: **platform + device model + OS version**. -OS defaults to the newest runtime already installed on the machine. If the -requested runtime / system image is not installed, the lease fails with a -clear error unless `--allow-download` is passed (downloads are multi-GB and -must never be triggered implicitly). +OS defaults to the newest runtime already installed on the machine that can +actually run the requested model — for iOS specifically, the newest +installed runtime that both falls inside the device type's supported range +(`simctl list devicetypes`' `minRuntimeVersion`/`maxRuntimeVersion`) and +still lists the model in its `supportedDeviceTypes`, not the newest +installed runtime overall (a newer runtime can drop a model, as iOS 26 did +for iPhone XS/XR). This still resolves on a fresh Xcode install with zero +simulator runtimes present at all — an empty runtime list is a normal +starting state, not a malformed catalog, so it falls straight through to +the same "not installed, permitted to download" path as a non-empty catalog +that simply lacks a matching runtime. If the requested runtime / system +image is not installed, the lease fails with a clear error unless downloads +are permitted for that request (downloads are multi-GB and must never be +triggered implicitly). An OS version outside a model's supported range +fails immediately with the range named in the error — never as an attempted +download, since no download could make it work. + +Permission comes from `config.downloads.policy`, resolved once, in the +daemon, before a request ever reaches the acquisition path: `"never"` +forbids installs outright, even over an explicit `--allow-download` / +`allow_download`; `"always"` grants it to every explicit lease request +without the caller having to ask; `"on-request"` (the default) defers to +the request's own flag, which is today's behavior byte-for-byte. Only an +explicit lease request (`LeaseEngine#request`) can carry download +permission to a driver's `resolveSpec` — warm-pool provisioning and startup +convergence reuse specs already committed to the registry and never call +`resolveSpec` themselves, so neither can trigger a download regardless of +policy. diff --git a/docs/CLI.md b/docs/CLI.md index 4be9040..11f3586 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -39,6 +39,8 @@ longer dumped to stderr on every failure, only on request via `--help`. | 12 | `NO_DRIVER` | no driver registered for the requested platform | | 12 | `RUNTIME_MISSING` | runtime not installed and no `--allow-download` | | 12 | `UNKNOWN_MODEL` | unknown device model for the platform | +| 12 | `INSUFFICIENT_DISK_SPACE` | not enough free disk space to install a component | +| 12 | `LICENSE_NOT_ACCEPTED` | a required license (e.g. an Android SDK license) is not accepted | | 13 | `REQUESTER_ALREADY_LEASED` | requester already holds a lease or has a pending request — one lease per agent in v1; release the named lease first | | 14 | — | `lease` held mode only: the daemon ended the lease without the holder asking (TTL backstop, operator `release`, or an unrecoverable device) | @@ -89,8 +91,24 @@ simlock lease --platform --device [--os ] - `--no-wait` — fail immediately with exit 11 instead of queueing. - `--allow-download` — permit downloading a missing runtime / system image (multi-GB; never implicit). Without it, a missing runtime is exit 12. - iOS runtimes remain Xcode-managed in v1: `--allow-download` cannot install - them; install the runtime through Xcode first. + For iOS, this runs `xcodebuild -downloadPlatform iOS` under the hood and + only reaches back to iOS 16.0 (a floor of Xcode's own downloader); older + runtimes and unknown device types (which need a newer Xcode) still require + installing/upgrading Xcode by hand. A requested `--os` outside the + device's supported runtime range (e.g. iPhone Xs above iOS 18.x) fails + immediately — no download is ever attempted for a version that could not + work regardless. For Android, this runs `sdkmanager --install`; an + unaccepted SDK license fails naming `downloads.acceptAndroidLicenses` + (config) unless that flag is set, in which case licenses are accepted + automatically and the install retried once. Both drivers check free disk + space before starting either install and fail fast, naming required vs. + available bytes, instead of risking a full disk mid-download. Every + install attempt (including a license-triggered retry) emits + `component.install-started` / `component.installed` / + `component.install-failed` on the event bus (`simlock events --follow`); + see [EVENTS.md](EVENTS.md#components). The requester's own progress stream + (below) does not yet reflect an in-flight download — see + [known-pitfalls.md](known-pitfalls.md). - `--detach` — detached mode: print the lease result and exit; the lease is TTL-bound and must be renewed with `simlock lease renew`. - `--bind-pid ` — held mode only: watch this pid for death instead of diff --git a/docs/EVENTS.md b/docs/EVENTS.md index 643d89d..24cc84b 100644 --- a/docs/EVENTS.md +++ b/docs/EVENTS.md @@ -40,6 +40,24 @@ in short: `subject.past-tense-fact`, emitted post-commit, facts not commands. | `device.recovered` | device id, lease id, attempts, duration | a crashed leased device was rebooted under its existing lease and passed readiness | LeaseHealthMonitor | implemented | | `device.recovery-failed` | device id, lease id, attempts, reason, error | recovery could not restore a leased device (absent from driver reality, provenance drift, or attempts exhausted) and its lease was released | LeaseHealthMonitor | implemented | +## Components + +| Event | Payload (key fields) | Emitted when | Emitter | Status | +|---|---|---|---|---| +| `component.install-started` | platform, component id (iOS runtime version or "latest"; Android `sdkmanager` package name), requester id (when the triggering resolution knew one) | a driver is about to run `xcodebuild -downloadPlatform` / `sdkmanager --install` for a missing component, disk preflight already passed | driver-diagnostics | implemented | +| `component.installed` | platform, component id, duration, requester id | the install succeeded **and** a post-install re-scan confirmed the component the request actually needed is present (paired with the requested device type, for iOS) — never fired on a bare exit-0 | driver-diagnostics | implemented | +| `component.install-failed` | platform, component id, duration, stable error summary, requester id | the install failed, including a license-retry failure (exactly one `install-failed` per attempted install, never one per retry), **or** the installer exited 0 but the post-install re-scan could not confirm the component | driver-diagnostics | implemented | + +Drivers never touch the event bus directly (architecture rule 5): both drivers report these +facts through their own `onDiagnostic` callback (mirroring the Android driver's pre-existing +diagnostic pattern), and `src/daemon/main.ts` bridges that diagnostic to the bus at driver +construction time — hence the `driver-diagnostics` emitter rather than `IosSimctlDriver` / +`AndroidDriver`. A disk-preflight failure (`InsufficientDiskSpaceError`) happens before any +diagnostic fires: no install was attempted, so nothing is reported as started or failed. See +"Device requests" and "Fresh-state strategy" in [ARCHITECTURE.md](ARCHITECTURE.md) for how a +missing component gets to this point, and `docs/known-pitfalls.md` for the requester-visible +progress gap this leaves. + ## System | Event | Payload (key fields) | Emitted when | Emitter | Status | diff --git a/docs/IDEAS.md b/docs/IDEAS.md index 951d141..d9d2e60 100644 --- a/docs/IDEAS.md +++ b/docs/IDEAS.md @@ -53,6 +53,34 @@ boundary held. (pre-installed certs, test apps). Offer per-pool config: reclaim by erase (default) or by re-clone from a maintained golden device. +## Parse Apple's downloadables index for exact runtime versions (iOS) + +The bounded-default download path (`IosSimctlDriver#resolveDefaultRuntime`, +`src/drivers/ios/index.ts`) can only ask `xcodebuild -downloadPlatform iOS +-buildVersion ` when no `--os` was given and the model's pairing range +has an upper bound — the bare major version, not an exact patch release, +because the exact downloadable versions for the installed Xcode aren't known +offline. Xcode's own downloadable-runtimes catalog (fetched by Xcode/App +Store internally) would let the driver resolve the exact newest compatible +patch release instead of gambling on the major matching a real build. Not +pursued in v1: parsing an undocumented, Apple-controlled catalog format is a +maintenance burden disproportionate to the edge case it closes (see +`docs/known-pitfalls.md`, "Component downloads"). + +## Requester-visible download progress + +Neither driver's install (`xcodebuild -downloadPlatform`, `sdkmanager +--install`) currently reaches the requesting connection's progress stream — +see `docs/known-pitfalls.md` ("no progress push"). Closing this needs a +`downloading` stage on `LeaseProgress` (`src/core/wait-queue.ts`) and a +`Driver.resolveSpec` progress callback both drivers implement, threaded +through `LeaseAcquisitionCoordinator#resolveAndDrive` the same way +`provision`/`makeReady` already report `provisioning`/`booting`. Deferred +because it is protocol machinery (interface + CLI/MCP wire changes), not a +small addition, and the daemon-side `component.install-started` bus event +already gives an operator visibility via `simlock events --follow` even +though the waiting requester itself does not see it yet. + ## Cross-machine coordination A fleet-level broker over multiple hosts. Explicitly out of scope for v1 diff --git a/docs/agent-rules/safety.md b/docs/agent-rules/safety.md index 61aea38..5efa169 100644 --- a/docs/agent-rules/safety.md +++ b/docs/agent-rules/safety.md @@ -27,7 +27,13 @@ never enforce them only inside an individual rule or driver. over a read-only registry view returning proposed actions. A rule that executes side effects directly is a bug regardless of what it does. 4. **No implicit multi-GB downloads.** Missing runtimes / system images fail - the request unless `--allow-download` was explicitly passed. + the request unless `--allow-download` (or MCP's `allow_download`) was + explicitly passed, or `downloads.policy: "always"` is set in config -- + both count as the required explicit consent, and `downloads.policy: + "never"` overrides either one back to forbidden. Warm-pool provisioning + and startup convergence never trigger a download under any policy: they + only ever reuse specs already committed to the registry, never resolve a + new one. 5. **Destructive CLI commands confirm or require `--yes`** (`release --all`, `nuke`). `cleanup` must always support `--dry-run`. 6. **Every destructive action is attributable.** Log/emit which rule or diff --git a/docs/known-pitfalls.md b/docs/known-pitfalls.md index 680dd57..d3b2f45 100644 --- a/docs/known-pitfalls.md +++ b/docs/known-pitfalls.md @@ -99,3 +99,50 @@ destroys it (registry-only, as always). The device stays visible as `device.purge-failed` still fires as before; `device.quarantined`, `device.quarantine-recovered`, and `device.quarantine-abandoned` are the new follow-up facts (see `docs/EVENTS.md`). + +## Component downloads: per-request blocking, the bounded-default edge case, and no progress push + +The iOS driver's `resolveSpec` (`src/drivers/ios/index.ts`) can now run +`xcodebuild -downloadPlatform iOS` when a requested runtime is missing and +downloads are permitted. Two things worth knowing about that path: + +**Only the requesting lease waits.** `resolveSpec` runs inside +`LeaseAcquisitionCoordinator#resolveAndDrive`, per request, outside the +serialized decision gate and outside the FIFO head — a slow download (tens +of minutes for a ~7 GB runtime) blocks only the request that triggered it. +Concurrent requests for the *same* missing runtime are deduped behind an +in-driver promise (one `xcodebuild` invocation, all callers await it); a +request for a different model or version proceeds independently and is +never queued behind someone else's download. + +**The bounded-default edge case.** When no `--os` is given and no installed +runtime pairs with the model, the driver has to guess a version to +download: unbounded models (no `maxRuntimeVersion` cap) get a plain +`-downloadPlatform iOS` (latest), but a model with a bounded max (like an +older device type whose newest compatible runtime is a specific release) +gets `-buildVersion ` — just the major +version number, since the exact patch release isn't known offline (Apple's +downloadables index isn't parsed in v1; see `docs/IDEAS.md`). If Xcode +doesn't have a build matching that bare major version, the download fails +and the caller is told to pass `--os ` explicitly rather than +retrying blind. + +**No requester-visible progress during a download (#67 stage 4).** The +requester's lease-progress stream (`LeaseProgress` in `src/core/wait-queue.ts` +— `queued` / `provisioning` / `booting` / `reclaiming`, relayed as CLI stderr +JSON lines and MCP `notifications/progress`) has no `downloading` stage. Both +drivers' `resolveSpec` — where a runtime or system-image install actually +happens — runs before `LeaseAcquisitionCoordinator#drive`'s provisioning +step, and `Driver.resolveSpec`'s signature carries no progress callback the +way `provision`/`makeReady` do. A held-mode CLI or MCP caller waiting on a +multi-minute install today sees nothing on the wire between its request and +either the eventual grant or a timeout; the only visibility is the daemon's +own `component.install-started` bus event (`simlock events --follow`) and log +line, neither reaching the waiting connection itself. Threading a +`downloading` stage through would mean widening the `Driver` interface +(`resolveSpec` gaining an `onProgress`-shaped option, both drivers +implementing it), a new `LeaseProgress` variant, and CLI/MCP wire changes — +real protocol machinery, not a small addition, so it was deliberately not +built in stage 4. `component.install-started`'s payload already carries +enough (`platform`, `componentId`) that a future pass wiring this through +would mostly be plumbing, not new information to invent. diff --git a/e2e/fake-driver/fake-driver.ts b/e2e/fake-driver/fake-driver.ts index 5e52492..1f8bf40 100644 --- a/e2e/fake-driver/fake-driver.ts +++ b/e2e/fake-driver/fake-driver.ts @@ -69,7 +69,7 @@ export class OutOfProcessFakeDriver implements Driver { async resolveSpec( request: DeviceRequest, - options: { readonly allowDownload: boolean }, + options: { readonly allowDownload: boolean; readonly requesterId?: string }, ): Promise { const script = await this.#beforeCall("resolveSpec", [request, options]); this.#assertKnownModel(request.model, script); diff --git a/src/bus/index.test.ts b/src/bus/index.test.ts index 95f0b07..009dadd 100644 --- a/src/bus/index.test.ts +++ b/src/bus/index.test.ts @@ -133,6 +133,9 @@ describe("EventBus", () => { | "device.quarantine-stranded" | "device.shutdown" | "device.deleted" + | "component.install-started" + | "component.installed" + | "component.install-failed" | "device.foreign-state-detected" | "device.foreign-provenance-detected" | "device.stalled-transition-detected" diff --git a/src/bus/index.ts b/src/bus/index.ts index 254eb3c..65606ff 100644 --- a/src/bus/index.ts +++ b/src/bus/index.ts @@ -86,6 +86,25 @@ export interface EventMap { }; "device.shutdown": { readonly deviceId: string; readonly initiator: string }; "device.deleted": { readonly deviceId: string; readonly initiator: string }; + "component.install-started": { + readonly platform: string; + readonly componentId: string; + /** The requester on whose behalf this install runs, when the triggering resolution knew one. */ + readonly requesterId?: string; + }; + "component.installed": { + readonly platform: string; + readonly componentId: string; + readonly durationMs: number; + readonly requesterId?: string; + }; + "component.install-failed": { + readonly platform: string; + readonly componentId: string; + readonly durationMs: number; + readonly error: string; + readonly requesterId?: string; + }; "daemon.started": { readonly version: string; readonly configSnapshot: unknown }; "daemon.stopping": { readonly reason: string }; "disk.pressure-detected": { readonly freeBytes: number; readonly threshold: number }; diff --git a/src/cli/index.test.ts b/src/cli/index.test.ts index 07ad286..ebc3e89 100644 --- a/src/cli/index.test.ts +++ b/src/cli/index.test.ts @@ -73,6 +73,8 @@ describe("CLI boundary", () => { ["NO_CAPACITY", 11], ["RUNTIME_MISSING", 12], ["UNKNOWN_MODEL", 12], + ["INSUFFICIENT_DISK_SPACE", 12], + ["LICENSE_NOT_ACCEPTED", 12], ["BAD_REQUEST", 2], ["REQUESTER_ALREADY_LEASED", 13], ] as const)("maps %s daemon errors to exit %d", async (code, expected) => { @@ -1046,6 +1048,7 @@ function testConfig(): Config { stableObservations: 2, }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, idle: { deleteAfterMs: 60_000, shutdownAfterMs: 10_000 }, lease: { detachedTtlMs: 60_000, heldTtlBackstopMs: 60_000, heartbeatIntervalMs: 15_000 }, capacity: { diff --git a/src/cli/index.ts b/src/cli/index.ts index 4e8b62b..ed47ec2 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -47,6 +47,8 @@ const LEASE_LOST_EXIT_CODE = 14; const DAEMON_ERROR_EXIT_CODES: Readonly> = { BAD_FRAME: 2, BAD_REQUEST: 2, + INSUFFICIENT_DISK_SPACE: 12, + LICENSE_NOT_ACCEPTED: 12, NO_CAPACITY: 11, NO_DRIVER: 12, QUEUE_TIMEOUT: 10, diff --git a/src/core/acquisition-planner.test.ts b/src/core/acquisition-planner.test.ts index 9eecea8..d13d9d6 100644 --- a/src/core/acquisition-planner.test.ts +++ b/src/core/acquisition-planner.test.ts @@ -21,6 +21,7 @@ const config: Config = { stableObservations: 2, }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, idle: { deleteAfterMs: 60_000, shutdownAfterMs: 10_000 }, warmPool: { quarantine: { diff --git a/src/core/cleanup/idle-destroy.test.ts b/src/core/cleanup/idle-destroy.test.ts index 67dd3c5..253cded 100644 --- a/src/core/cleanup/idle-destroy.test.ts +++ b/src/core/cleanup/idle-destroy.test.ts @@ -17,6 +17,7 @@ const config: Config = { stableObservations: 2, }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, idle: { deleteAfterMs: 30_000, shutdownAfterMs: 10_000 }, warmPool: { quarantine: { diff --git a/src/core/cleanup/idle-shutdown.test.ts b/src/core/cleanup/idle-shutdown.test.ts index ef3c800..5d305d1 100644 --- a/src/core/cleanup/idle-shutdown.test.ts +++ b/src/core/cleanup/idle-shutdown.test.ts @@ -15,6 +15,7 @@ const config: Config = { stableObservations: 2, }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, idle: { deleteAfterMs: 30_000, shutdownAfterMs: 10_000 }, warmPool: { quarantine: { diff --git a/src/core/config.test.ts b/src/core/config.test.ts index a639514..266be8c 100644 --- a/src/core/config.test.ts +++ b/src/core/config.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from "vitest"; import { MemoryFilesystem, FakeSystemStats } from "../ports/index.js"; import type { ResourceStrategyOptions } from "./capacity/index.js"; -import { type Config, loadConfig } from "./index.js"; +import { type Config, effectiveAllowDownload, loadConfig } from "./index.js"; /** Narrows the capacity block for assertions on resource-strategy configs. */ function resourceOptions(config: Config): ResourceStrategyOptions { @@ -72,6 +72,7 @@ describe("loadConfig", () => { }, }, log: { level: "info", rotateBytes: 5 * 1024 * 1024 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, warmPool: { quarantine: { maxRetries: 3, @@ -356,6 +357,81 @@ describe("loadConfig", () => { expect(warn).toHaveBeenCalledWith('Unknown config key: "health.maxBoltCount"'); }); + it("applies a file-level downloads override", async () => { + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/agent/.simlock"); + await filesystem.writeFileAtomic( + configPath, + JSON.stringify({ + downloads: { policy: "always", acceptAndroidLicenses: true, timeoutMs: 60_000 }, + }), + ); + + const config = await loadConfig({ configPath, filesystem, systemStats: createStats() }); + expect(config.downloads).toEqual({ + policy: "always", + acceptAndroidLicenses: true, + timeoutMs: 60_000, + }); + }); + + it("applies an override-level downloads policy over the file value", async () => { + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/agent/.simlock"); + await filesystem.writeFileAtomic( + configPath, + JSON.stringify({ downloads: { policy: "never" } }), + ); + + const config = await loadConfig({ + configPath, + filesystem, + overrides: { downloads: { policy: "always" } }, + systemStats: createStats(), + }); + expect(config.downloads.policy).toBe("always"); + }); + + it("rejects a downloads.policy outside the known set", async () => { + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/agent/.simlock"); + await filesystem.writeFileAtomic( + configPath, + JSON.stringify({ downloads: { policy: "sometimes" } }), + ); + + await expect( + loadConfig({ configPath, filesystem, systemStats: createStats() }), + ).rejects.toThrow("downloads.policy"); + }); + + it("rejects a non-boolean downloads.acceptAndroidLicenses", async () => { + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/agent/.simlock"); + await filesystem.writeFileAtomic( + configPath, + JSON.stringify({ downloads: { acceptAndroidLicenses: "yes" } }), + ); + + await expect( + loadConfig({ configPath, filesystem, systemStats: createStats() }), + ).rejects.toThrow("downloads.acceptAndroidLicenses"); + }); + + it.each([ + [{ downloads: { timeoutMs: 0 } }, "downloads.timeoutMs"], + [{ downloads: { timeoutMs: -1 } }, "downloads.timeoutMs"], + [{ downloads: { timeoutMs: "1200000" } }, "downloads.timeoutMs"], + ])("rejects a non-positive or malformed downloads.timeoutMs", 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("applies a file-level stalledTransition override", async () => { const filesystem = new MemoryFilesystem(); await filesystem.mkdirp("/home/agent/.simlock"); @@ -536,3 +612,20 @@ describe("loadConfig legacy capacity keys", () => { expect(warn).toHaveBeenCalledWith(expect.stringContaining("Ignoring limits")); }); }); + +describe("effectiveAllowDownload", () => { + it("grants downloads for every request under the always policy", () => { + expect(effectiveAllowDownload("always", false)).toBe(true); + expect(effectiveAllowDownload("always", true)).toBe(true); + }); + + it("forbids downloads for every request under the never policy, even an explicit true", () => { + expect(effectiveAllowDownload("never", false)).toBe(false); + expect(effectiveAllowDownload("never", true)).toBe(false); + }); + + it("defers to the request's own flag under the on-request policy", () => { + expect(effectiveAllowDownload("on-request", false)).toBe(false); + expect(effectiveAllowDownload("on-request", true)).toBe(true); + }); +}); diff --git a/src/core/config.ts b/src/core/config.ts index b426c14..4dee9af 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -28,8 +28,23 @@ import { const DEFAULT_CONFIG_PATH = "~/.simlock/config.json"; +/** + * `never` forbids installs even when a request passes `--allow-download` (locked-down + * machines/CI). `on-request` (default) preserves today's contract: install only when the + * request itself carries the flag. `always` lets the daemon install missing components for + * any explicit lease request without the per-request flag. + */ +export type DownloadPolicy = "never" | "on-request" | "always"; + export interface Config { readonly capacity: CapacityConfig; + readonly downloads: { + readonly policy: DownloadPolicy; + /** Explicit legal consent for Android SDK licenses, independent of `policy`. */ + readonly acceptAndroidLicenses: boolean; + /** Per-install timeout; downloads run minutes, not seconds. */ + readonly timeoutMs: number; + }; readonly idle: { readonly shutdownAfterMs: number; readonly deleteAfterMs: number; @@ -131,6 +146,19 @@ export async function loadConfig({ return deepFreeze(merged); } +/** + * Reduces `downloads.policy` and a request's own `--allow-download` / `allow_download` flag + * to the single permission a driver's `resolveSpec` actually sees. `never` overrides an + * explicit `true` on the request -- the whole point of the policy is that it cannot be + * opted back into per request -- and `always` grants permission the request never had to ask + * for. Only `on-request` defers to what the caller asked for, which is today's behavior. + */ +export function effectiveAllowDownload(policy: DownloadPolicy, requested: boolean): boolean { + if (policy === "always") return true; + if (policy === "never") return false; + return requested; +} + /** * Cross-field check: the heartbeat cadence must be frequent enough, relative to the * backstop, that a few missed beats (context compaction, a slow tick) still land well @@ -220,6 +248,11 @@ function defaultConfig(systemStats: SystemStats, strategy: CapacityStrategyName) strategy, config: defaultCapacityOptions(strategy, systemStats), } as CapacityConfig, + downloads: { + policy: "on-request", + acceptAndroidLicenses: false, + timeoutMs: 1_200_000, + }, idle: { shutdownAfterMs: 10 * 60_000, deleteAfterMs: 60 * 60_000, @@ -296,6 +329,7 @@ function validateConfigLayer( } const LOG_LEVELS: readonly LogLevel[] = ["debug", "info", "warn", "error"]; +const DOWNLOAD_POLICIES: readonly DownloadPolicy[] = ["never", "on-request", "always"]; /** * The `capacity.config` validator is the selected strategy's own, so a strategy @@ -310,6 +344,11 @@ function configValidators(strategy: CapacityStrategyName): Record minimumThresholdMs: 1_000, ...stalledTransitionOverrides, }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, }; } diff --git a/src/core/driver-catalog.ts b/src/core/driver-catalog.ts index 9f1a06a..dc0f346 100644 --- a/src/core/driver-catalog.ts +++ b/src/core/driver-catalog.ts @@ -30,7 +30,7 @@ export class DriverCatalog { async resolveSpec( request: DeviceRequest, - options: { readonly allowDownload: boolean }, + options: { readonly allowDownload: boolean; readonly requesterId?: string }, ): Promise { return this.get(request.platform).resolveSpec(request, options); } diff --git a/src/core/driver.test.ts b/src/core/driver.test.ts new file mode 100644 index 0000000..cb5ce2e --- /dev/null +++ b/src/core/driver.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from "vitest"; + +import { MemoryFilesystem } from "../ports/index.js"; +import { assertDiskSpace, DiskSpaceGuard, InsufficientDiskSpaceError } from "./driver.js"; + +const gibibyte = 1024 ** 3; + +describe("assertDiskSpace", () => { + it("resolves when free space covers the requirement", async () => { + const filesystem = new MemoryFilesystem(10 * gibibyte); + + await expect( + assertDiskSpace(filesystem, "ios", 6 * gibibyte, "/volume"), + ).resolves.toBeUndefined(); + }); + + it("throws InsufficientDiskSpaceError naming required and available bytes when it doesn't", async () => { + const filesystem = new MemoryFilesystem(4 * gibibyte); + + const error = await assertDiskSpace(filesystem, "android", 6 * gibibyte, "/volume").catch( + (caught: unknown) => caught, + ); + + expect(error).toBeInstanceOf(InsufficientDiskSpaceError); + expect(error).toMatchObject({ + availableBytes: 4 * gibibyte, + platform: "android", + requiredBytes: 6 * gibibyte, + }); + }); +}); + +describe("DiskSpaceGuard", () => { + it("lets a single reservation through when it fits free space", async () => { + const filesystem = new MemoryFilesystem(10 * gibibyte); + const guard = new DiskSpaceGuard(); + + await expect(guard.reserve(filesystem, "ios", 6 * gibibyte, "/volume")).resolves.toBeInstanceOf( + Function, + ); + }); + + it("rejects a reservation that alone exceeds free space, with InsufficientDiskSpaceError", async () => { + const filesystem = new MemoryFilesystem(4 * gibibyte); + const guard = new DiskSpaceGuard(); + + await expect(guard.reserve(filesystem, "ios", 6 * gibibyte, "/volume")).rejects.toBeInstanceOf( + InsufficientDiskSpaceError, + ); + }); + + it("rejects a second concurrent reservation that would overfill the volume alongside the first", async () => { + const filesystem = new MemoryFilesystem(10 * gibibyte); + const guard = new DiskSpaceGuard(); + + // First reservation fits (6 of 10 GiB); still outstanding when the second is attempted. + const releaseFirst = await guard.reserve(filesystem, "ios", 6 * gibibyte, "/volume"); + + // 10 GiB free minus the 6 GiB already reserved leaves 4 GiB -- not enough for another 6 GiB, + // even though a bare disk-free reading alone would say yes. + await expect( + guard.reserve(filesystem, "android", 6 * gibibyte, "/volume"), + ).rejects.toBeInstanceOf(InsufficientDiskSpaceError); + + releaseFirst(); + }); + + it("frees the reservation on release, letting a subsequent reservation succeed", async () => { + const filesystem = new MemoryFilesystem(10 * gibibyte); + const guard = new DiskSpaceGuard(); + + const releaseFirst = await guard.reserve(filesystem, "ios", 6 * gibibyte, "/volume"); + releaseFirst(); + + await expect( + guard.reserve(filesystem, "android", 6 * gibibyte, "/volume"), + ).resolves.toBeInstanceOf(Function); + }); + + it("tracks reservations independently per path", async () => { + const filesystem = new MemoryFilesystem(10 * gibibyte); + const guard = new DiskSpaceGuard(); + + // Both reservations are 6 of the same 10 GiB free reading, but against different paths -- + // neither should see the other's outstanding bytes. + await expect( + guard.reserve(filesystem, "ios", 6 * gibibyte, "/volume-a"), + ).resolves.toBeInstanceOf(Function); + await expect( + guard.reserve(filesystem, "android", 6 * gibibyte, "/volume-b"), + ).resolves.toBeInstanceOf(Function); + }); + + it("is idempotent: releasing twice does not double-free the reservation", async () => { + const filesystem = new MemoryFilesystem(10 * gibibyte); + const guard = new DiskSpaceGuard(); + + const release = await guard.reserve(filesystem, "ios", 6 * gibibyte, "/volume"); + release(); + release(); + + // A double release must not credit the 6 GiB back twice, which would let two more 6 GiB + // reservations both succeed against only 10 GiB of real free space. + const releaseSecond = await guard.reserve(filesystem, "ios", 6 * gibibyte, "/volume"); + await expect( + guard.reserve(filesystem, "android", 6 * gibibyte, "/volume"), + ).rejects.toBeInstanceOf(InsufficientDiskSpaceError); + releaseSecond(); + }); +}); diff --git a/src/core/driver.ts b/src/core/driver.ts index 022f67f..c371d22 100644 --- a/src/core/driver.ts +++ b/src/core/driver.ts @@ -1,3 +1,4 @@ +import type { Filesystem } from "../ports/index.js"; import type { DeviceSpec, Platform } from "./domain.js"; export interface DeviceRequest { @@ -100,7 +101,16 @@ export interface Driver { readonly platform: Platform; resolveSpec( request: DeviceRequest, - options: { readonly allowDownload: boolean }, + options: { + readonly allowDownload: boolean; + /** + * The requester on whose behalf this resolution runs, when known. Optional: a caller that + * resolves outside of a lease request (e.g. a driver revalidating its own cached spec) has + * no requester to attribute. Threaded through to a driver's component-install diagnostics + * so the resulting `component.install-*` events carry it -- see `docs/EVENTS.md`. + */ + readonly requesterId?: string; + }, ): Promise; provision(spec: DeviceSpec): Promise; /** @@ -124,12 +134,24 @@ export interface Driver { } export class RuntimeMissingError extends Error { + /** + * Whether a download could plausibly fix this. `true` by default -- a plain "runtime not + * installed" is exactly what `--allow-download` exists for. A subclass reporting a request + * no download can ever satisfy (out of the model's pairing range, an installed runtime that + * does not pair with the model, a version older than Xcode's automatic-download floor) sets + * this `false` so callers (see the daemon's download-policy suffix) don't point someone at a + * flag that cannot help. + */ + readonly downloadable: boolean; + constructor( readonly platform: Platform, readonly osVersion: string, + options?: { readonly downloadable?: boolean }, ) { super(`Runtime missing for ${platform} ${osVersion}`); this.name = "RuntimeMissingError"; + this.downloadable = options?.downloadable ?? true; } } @@ -156,3 +178,116 @@ export class DriverCrashError extends Error { this.name = "DriverCrashError"; } } + +export class InsufficientDiskSpaceError extends Error { + constructor( + readonly platform: Platform, + readonly requiredBytes: number, + readonly availableBytes: number, + ) { + super( + `Not enough free disk space to install a ${platform} component: needs ~` + + `${formatGibibytes(requiredBytes)} free, only ${formatGibibytes(availableBytes)} available`, + ); + this.name = "InsufficientDiskSpaceError"; + } +} + +function formatGibibytes(bytes: number): string { + return `${(bytes / 1024 ** 3).toFixed(1)} GiB`; +} + +/** + * A component install was refused because a required license/EULA is not accepted -- + * platform-agnostic the same way `RuntimeMissingError` is, so the daemon can map it to a + * stable error code without importing a driver module. `AndroidLicenseNotAcceptedError` (the + * only concrete case today) extends this with its own message; a future platform with the same + * shape of gate would do the same rather than the daemon special-casing Android. + */ +export class LicenseNotAcceptedError extends Error { + constructor( + readonly platform: Platform, + readonly componentName: string, + ) { + super(`A license required to install ${componentName} for ${platform} is not accepted`); + this.name = "LicenseNotAcceptedError"; + } +} + +/** + * Serializes disk-space preflight across concurrent component installs sharing a volume. + * `assertDiskSpace` alone only ever sees the disk's free space at the instant it is called: two + * installs racing the same preflight (an iOS runtime download and an Android system-image + * install, or two of either) can each observe enough free space and both proceed, jointly + * overfilling the volume neither alone would have. A single shared `DiskSpaceGuard` instance, + * injected into every driver that installs components (wired once in `src/daemon/main.ts`), + * fixes that by tracking bytes reserved but not yet released, keyed per path, and checking free + * space *minus* those outstanding reservations rather than free space alone. + * + * `reserve` resolves or throws synchronously with respect to any other in-flight `reserve` call: + * the only `await` is `filesystem.diskFree`, and the check-then-record step immediately after it + * runs to completion before any other queued continuation gets a turn (JS's single-threaded + * run-to-completion semantics), so two concurrent reservations against the same path can never + * both observe headroom the other has already claimed. + */ +export class DiskSpaceGuard { + readonly #outstandingBytesByPath = new Map(); + + /** + * Reserves `requiredBytes` against `path`'s free space, minus whatever this guard already has + * outstanding there. Throws `InsufficientDiskSpaceError` (same shape `assertDiskSpace` throws) + * when the reservation would not fit. On success, returns a release function the caller must + * invoke exactly once (typically in a `finally`) once the install this reservation was made + * for has settled, freeing the bytes for the next reservation. + */ + async reserve( + filesystem: Pick, + platform: Platform, + requiredBytes: number, + path = ".", + ): Promise<() => void> { + const availableBytes = await filesystem.diskFree(path); + const outstandingBytes = this.#outstandingBytesByPath.get(path) ?? 0; + const effectivelyAvailableBytes = availableBytes - outstandingBytes; + if (effectivelyAvailableBytes < requiredBytes) { + throw new InsufficientDiskSpaceError( + platform, + requiredBytes, + Math.max(0, effectivelyAvailableBytes), + ); + } + this.#outstandingBytesByPath.set(path, outstandingBytes + requiredBytes); + + let released = false; + return () => { + if (released) return; + released = true; + const remaining = (this.#outstandingBytesByPath.get(path) ?? 0) - requiredBytes; + if (remaining <= 0) { + this.#outstandingBytesByPath.delete(path); + } else { + this.#outstandingBytesByPath.set(path, remaining); + } + }; + } +} + +/** + * Checked before a driver starts any multi-GB component download/install, so a full disk fails + * fast with a clear message instead of filling up mid-download (see safety rule 4's spirit -- + * downloads must never surprise the machine they run on). `path` defaults to `"."`, the same + * convention `CleanupReaper` uses for its own disk-pressure check (`src/core/reaper.ts`): the + * daemon process's own working-directory volume. Single-shot: does not account for another + * concurrent install's own in-flight reservation -- see `DiskSpaceGuard` for that. + */ +export async function assertDiskSpace( + filesystem: Pick, + platform: Platform, + requiredBytes: number, + path = ".", +): Promise { + const availableBytes = await filesystem.diskFree(path); + if (availableBytes < requiredBytes) { + throw new InsufficientDiskSpaceError(platform, requiredBytes, availableBytes); + } +} diff --git a/src/core/fake-driver.ts b/src/core/fake-driver.ts index d5afe7c..b8792e3 100644 --- a/src/core/fake-driver.ts +++ b/src/core/fake-driver.ts @@ -93,7 +93,7 @@ export class FakeDriver implements Driver { async resolveSpec( request: DeviceRequest, - options: { readonly allowDownload: boolean }, + options: { readonly allowDownload: boolean; readonly requesterId?: string }, ): Promise { await this.#beforeCall("resolveSpec", request, options); this.#assertMatchingPlatform(request.platform); diff --git a/src/core/index.ts b/src/core/index.ts index 6233662..0870c4e 100644 --- a/src/core/index.ts +++ b/src/core/index.ts @@ -1,4 +1,11 @@ -export { type Config, type ConfigOverrides, loadConfig } from "./config.js"; +export { + type Config, + type ConfigOverrides, + // fallow-ignore-next-line unused-type -- public Config surface (config.downloads.policy); no in-tree consumer names it directly yet + type DownloadPolicy, + effectiveAllowDownload, + loadConfig, +} from "./config.js"; export { type DeviceRecord, type DeviceSpec, @@ -13,12 +20,15 @@ export { CleanupReaper } from "./reaper.js"; export { BootTimeoutError, type DeviceRequest, + DiskSpaceGuard, type Driver, type DriverCatalogEntry, DriverCrashError, type DriverDevice, type DriverEstimate, type DriverReality, + InsufficientDiskSpaceError, + LicenseNotAcceptedError, type ObservedDevice, type ObservedRunState, RuntimeMissingError, diff --git a/src/core/lease-acquisition-coordinator.test.ts b/src/core/lease-acquisition-coordinator.test.ts index 8237eb9..28eac33 100644 --- a/src/core/lease-acquisition-coordinator.test.ts +++ b/src/core/lease-acquisition-coordinator.test.ts @@ -26,6 +26,7 @@ const request = { model: "iPhone 16", osVersion: "26.5", platform: "ios" } as co function config(maxDevices = 1): Config { return { diskPressure: { freeBytesThreshold: 10 * gibibyte }, + downloads: { acceptAndroidLicenses: false, policy: "on-request", timeoutMs: 1_200_000 }, eventBuffer: { capacity: 100 }, health: { enabled: true, diff --git a/src/core/lease-acquisition-coordinator.ts b/src/core/lease-acquisition-coordinator.ts index 68d2c3f..31975d2 100644 --- a/src/core/lease-acquisition-coordinator.ts +++ b/src/core/lease-acquisition-coordinator.ts @@ -211,6 +211,11 @@ export class LeaseAcquisitionCoordinator implements AcquisitionMaintenance { try { waiter.spec = await driver.resolveSpec(request, { allowDownload: options.allowDownload ?? false, + // The waiter is the one place that knows which requester triggered this resolution; + // threaded through so a component install a driver ends up doing on this request's + // behalf can attribute its diagnostics (and the resulting `component.install-*` events) + // to it. + requesterId: options.requesterId, }); } catch (error: unknown) { await this.options.decisions.run(async () => { diff --git a/src/core/lease-engine.test.ts b/src/core/lease-engine.test.ts index 73c7120..4ba7734 100644 --- a/src/core/lease-engine.test.ts +++ b/src/core/lease-engine.test.ts @@ -22,6 +22,7 @@ const request = { model: "iPhone 16", osVersion: "26.5", platform: "ios" } as co function config(overrides: Partial = {}): Config { return { diskPressure: { freeBytesThreshold: 10 * gibibyte }, + downloads: { acceptAndroidLicenses: false, policy: "on-request", timeoutMs: 1_200_000 }, eventBuffer: { capacity: 100 }, health: { enabled: true, diff --git a/src/core/lease-health-monitor.test.ts b/src/core/lease-health-monitor.test.ts index c4291fb..7b36d00 100644 --- a/src/core/lease-health-monitor.test.ts +++ b/src/core/lease-health-monitor.test.ts @@ -51,6 +51,7 @@ function config(overrides: Partial = {}): Config { }, log: { level: "info", rotateBytes: 5 * 1024 * 1024 }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, }; } diff --git a/src/core/nuke.test.ts b/src/core/nuke.test.ts index 6101e7c..449dfdb 100644 --- a/src/core/nuke.test.ts +++ b/src/core/nuke.test.ts @@ -156,6 +156,7 @@ function config(): Config { stableObservations: 2, }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, idle: { deleteAfterMs: 10, shutdownAfterMs: 5 }, lease: { detachedTtlMs: 60_000, heldTtlBackstopMs: 60_000, heartbeatIntervalMs: 15_000 }, capacity: { diff --git a/src/core/quarantine-coordinator.ts b/src/core/quarantine-coordinator.ts index 2e3aa30..2572f5a 100644 --- a/src/core/quarantine-coordinator.ts +++ b/src/core/quarantine-coordinator.ts @@ -3,6 +3,7 @@ import type { Clock, TimerHandle } from "../ports/index.js"; import type { DeviceRecord, Platform } from "./domain.js"; import type { Driver, DriverDevice } from "./driver.js"; import type { SerializedDecision } from "./serialized-decision.js"; +import { stableError } from "./stable-error.js"; export interface QuarantineDriverCatalog { get(platform: Platform): Driver; @@ -238,11 +239,6 @@ function backoffDelay(config: QuarantineRetryConfig, attemptsSoFar: number): num ); } -function stableError(error: unknown): string { - const value = error instanceof Error ? error : new Error(String(error)); - return `${value.name}: ${value.message}`; -} - /** * `address` is never trusted by a driver's `reclaim` / `destroy` -- they derive what they need * from `driverData` -- so a quarantined device that has no address yet is not a lie a driver diff --git a/src/core/reaper.test.ts b/src/core/reaper.test.ts index 4984d3b..bdd4487 100644 --- a/src/core/reaper.test.ts +++ b/src/core/reaper.test.ts @@ -55,6 +55,7 @@ function config(): Config { stableObservations: 2, }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, idle: { deleteAfterMs: 30_000, shutdownAfterMs: 10_000 }, lease: { detachedTtlMs: 100, heldTtlBackstopMs: 100, heartbeatIntervalMs: 25 }, capacity: { diff --git a/src/core/stable-error.ts b/src/core/stable-error.ts new file mode 100644 index 0000000..1c35329 --- /dev/null +++ b/src/core/stable-error.ts @@ -0,0 +1,11 @@ +/** + * Renders any thrown value as `: ` -- the stable, human-readable summary + * shared by every `component.install-*` / `device.purge-failed`-style diagnostic that reports + * a caught error. Non-`Error` throws are wrapped first so every summary still has a name to + * report. One definition shared by both driver modules and the core coordinators that need it, + * rather than four copies drifting independently. + */ +export function stableError(error: unknown): string { + const value = error instanceof Error ? error : new Error(String(error)); + return `${value.name}: ${value.message}`; +} diff --git a/src/core/warm-pool-coordinator.test.ts b/src/core/warm-pool-coordinator.test.ts index fa1a630..522e2db 100644 --- a/src/core/warm-pool-coordinator.test.ts +++ b/src/core/warm-pool-coordinator.test.ts @@ -26,6 +26,7 @@ const config: Config = { stableObservations: 2, }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { policy: "on-request", acceptAndroidLicenses: false, timeoutMs: 1_200_000 }, idle: { deleteAfterMs: 60_000, shutdownAfterMs: 10_000 }, warmPool: { quarantine: { diff --git a/src/core/warm-pool-coordinator.ts b/src/core/warm-pool-coordinator.ts index 064577c..cf8dc7f 100644 --- a/src/core/warm-pool-coordinator.ts +++ b/src/core/warm-pool-coordinator.ts @@ -13,6 +13,7 @@ import type { Driver, DriverDevice } from "./driver.js"; import type { QuarantinePurgeFailure } from "./quarantine-coordinator.js"; import type { ReleasedLease } from "./registry.js"; import type { SerializedDecision } from "./serialized-decision.js"; +import { stableError } from "./stable-error.js"; export interface WarmPoolDriverCatalog { get(platform: Platform): Driver; @@ -211,11 +212,6 @@ function capacityDevices(devices: readonly DeviceRecord[]): readonly CapacityDev return devices.map((device) => ({ platform: device.spec.platform, state: device.state })); } -function stableError(error: unknown): string { - const value = error instanceof Error ? error : new Error(String(error)); - return `${value.name}: ${value.message}`; -} - /** * `address` is never trusted by a driver's `shutdown` / `reclaim` / `makeReady` -- they derive * whatever they need from `driverData` -- so a placeholder here is harmless. diff --git a/src/daemon/main.test.ts b/src/daemon/main.test.ts index afac58d..92c7e4b 100644 --- a/src/daemon/main.test.ts +++ b/src/daemon/main.test.ts @@ -4,6 +4,7 @@ import { join } from "node:path"; import { connect } from "node:net"; import { afterEach, describe, expect, it } from "vitest"; +import { EventBus } from "../bus/index.js"; import { FakeDriver } from "../core/index.js"; import { DAEMON_PROTOCOL_VERSION } from "../daemon-protocol/index.js"; import { @@ -14,7 +15,14 @@ import { MemoryLogSink, ScriptedProcessRunner, } from "../ports/index.js"; -import { discoverDrivers, startDaemon, type StartDaemonOptions } from "./main.js"; +import { + bridgeAndroidDriverDiagnostic, + discoverDrivers, + emitComponentInstallDiagnostic, + startDaemon, + wireComponentInstallLogging, + type StartDaemonOptions, +} from "./main.js"; import type { DaemonServer } from "./server.js"; const runningDaemons: DaemonServer[] = []; @@ -149,6 +157,7 @@ describe("discoverDrivers", () => { const drivers = await discoverDrivers({ clock: new FakeClock(), + eventBus: new EventBus(new FakeClock()), filesystem, idGenerator: new CryptoIdGenerator(), logger, @@ -166,6 +175,173 @@ describe("discoverDrivers", () => { }); }); +describe("component install diagnostic bridging", () => { + it("emits component.install-started/-installed/-failed for the bridged platform", () => { + const clock = new FakeClock(1_000); + const eventBus = new EventBus(clock); + const seen: unknown[] = []; + eventBus.subscribeAll((envelope) => seen.push(envelope)); + const bridge = emitComponentInstallDiagnostic(eventBus, "ios"); + + bridge({ componentId: "18.6", kind: "component-install-started" }); + bridge({ componentId: "18.6", durationMs: 42_000, kind: "component-installed" }); + bridge({ + componentId: "18.6", + durationMs: 5_000, + error: "DriverCrashError: xcodebuild failed", + kind: "component-install-failed", + }); + + expect(seen).toEqual([ + expect.objectContaining({ + event: "component.install-started", + module: "driver-diagnostics", + payload: { componentId: "18.6", platform: "ios" }, + }), + expect.objectContaining({ + event: "component.installed", + module: "driver-diagnostics", + payload: { componentId: "18.6", durationMs: 42_000, platform: "ios" }, + }), + expect.objectContaining({ + event: "component.install-failed", + module: "driver-diagnostics", + payload: { + componentId: "18.6", + durationMs: 5_000, + error: "DriverCrashError: xcodebuild failed", + platform: "ios", + }, + }), + ]); + }); + + it("carries requesterId onto the bridged event when the diagnostic knows one, and omits it when it doesn't", () => { + const clock = new FakeClock(1_000); + const eventBus = new EventBus(clock); + const seen: unknown[] = []; + eventBus.subscribeAll((envelope) => seen.push(envelope)); + const bridge = emitComponentInstallDiagnostic(eventBus, "ios"); + + bridge({ componentId: "18.6", kind: "component-install-started", requesterId: "agent-1" }); + bridge({ componentId: "18.6", kind: "component-install-started" }); + + expect(seen).toEqual([ + expect.objectContaining({ + event: "component.install-started", + payload: { componentId: "18.6", platform: "ios", requesterId: "agent-1" }, + }), + expect.objectContaining({ + event: "component.install-started", + payload: { componentId: "18.6", platform: "ios" }, + }), + ]); + }); + + it("forwards only component-install-* diagnostics from the Android driver's broader onDiagnostic surface", () => { + const clock = new FakeClock(1_000); + const eventBus = new EventBus(clock); + const seen: unknown[] = []; + eventBus.subscribeAll((envelope) => seen.push(envelope)); + const bridge = bridgeAndroidDriverDiagnostic(eventBus); + + bridge({ avdName: "simlock_1", kind: "snapshot-cold-boot", readyAfterMs: 15_000 }); + bridge({ + kind: "device-profile-source-unreadable", + path: "/x/.android/devices.xml", + reason: "parse-error", + }); + bridge({ + componentId: "system-images;android-35;google_apis;arm64-v8a", + kind: "component-install-started", + }); + + expect(seen).toEqual([ + expect.objectContaining({ + event: "component.install-started", + payload: { + componentId: "system-images;android-35;google_apis;arm64-v8a", + platform: "android", + }, + }), + ]); + }); +}); + +describe("wireComponentInstallLogging", () => { + it('writes a durable structured log line under logger.child("components") when component.installed fires', () => { + const clock = new FakeClock(1_000); + const sink = new MemoryLogSink(); + const logger = new JsonLinesLogger({ clock, level: "debug", sink }); + const eventBus = new EventBus(clock); + + wireComponentInstallLogging(eventBus, logger); + eventBus.emit( + "component.installed", + { componentId: "18.6", durationMs: 42_000, platform: "ios" }, + "driver-diagnostics", + ); + + expect(sink.records).toContainEqual( + expect.objectContaining({ + level: "info", + message: "Component installed", + module: "daemon.components", + fields: { componentId: "18.6", durationMs: 42_000, platform: "ios" }, + }), + ); + }); + + it("includes requesterId in the durable log line when the event carries one", () => { + const clock = new FakeClock(1_000); + const sink = new MemoryLogSink(); + const logger = new JsonLinesLogger({ clock, level: "debug", sink }); + const eventBus = new EventBus(clock); + + wireComponentInstallLogging(eventBus, logger); + eventBus.emit( + "component.installed", + { componentId: "18.6", durationMs: 42_000, platform: "ios", requesterId: "agent-1" }, + "driver-diagnostics", + ); + + expect(sink.records).toContainEqual( + expect.objectContaining({ + level: "info", + message: "Component installed", + module: "daemon.components", + fields: { + componentId: "18.6", + durationMs: 42_000, + platform: "ios", + requesterId: "agent-1", + }, + }), + ); + }); + + it("does not log for component.install-started or component.install-failed", () => { + const clock = new FakeClock(1_000); + const sink = new MemoryLogSink(); + const logger = new JsonLinesLogger({ clock, level: "debug", sink }); + const eventBus = new EventBus(clock); + + wireComponentInstallLogging(eventBus, logger); + eventBus.emit( + "component.install-started", + { componentId: "18.6", platform: "ios" }, + "driver-diagnostics", + ); + eventBus.emit( + "component.install-failed", + { componentId: "18.6", durationMs: 1_000, error: "boom", platform: "ios" }, + "driver-diagnostics", + ); + + expect(sink.records).toEqual([]); + }); +}); + describe("discoverDrivers with SIMLOCK_DRIVERS_MODULE", () => { const previousModule = process.env.SIMLOCK_DRIVERS_MODULE; @@ -189,6 +365,7 @@ describe("discoverDrivers with SIMLOCK_DRIVERS_MODULE", () => { const logger = new JsonLinesLogger({ clock: new FakeClock(), level: "debug", sink }); return discoverDrivers({ clock: new FakeClock(), + eventBus: new EventBus(new FakeClock()), filesystem: new MemoryFilesystem(), idGenerator: new CryptoIdGenerator(), logger, diff --git a/src/daemon/main.ts b/src/daemon/main.ts index 459f372..1ec3a59 100644 --- a/src/daemon/main.ts +++ b/src/daemon/main.ts @@ -7,13 +7,19 @@ import { type ConfigOverrides, type Driver, CleanupReaper, + DiskSpaceGuard, Doctor, LeaseEngine, loadConfig, Registry, Nuke, } from "../core/index.js"; -import { AndroidDriver, SdkMissingError } from "../drivers/android/index.js"; +import { + AndroidDriver, + SdkMissingError, + type AndroidDriverDiagnostic, +} from "../drivers/android/index.js"; +import type { ComponentInstallDiagnostic } from "../drivers/diagnostics.js"; import { IosSimctlDriver } from "../drivers/ios/index.js"; import { CryptoIdGenerator, @@ -85,10 +91,27 @@ export async function startDaemon(options: StartDaemonOptions = {}): Promise; readonly filesystem: Filesystem; readonly idGenerator: IdGenerator; readonly logger: Logger; @@ -182,13 +222,20 @@ export async function discoverDrivers(options: DriverDiscoveryContext): Promise< return loadDriversModule(driversModule, options, logger); } + const diskSpaceGuard = options.diskSpaceGuard ?? new DiskSpaceGuard(); const drivers: Driver[] = []; if (process.platform === "darwin") { drivers.push( new IosSimctlDriver({ clock: options.clock, + coreSimulatorRoot: `${homedir()}/Library/Developer/CoreSimulator`, + diskSpaceGuard, + ...(options.downloadTimeoutMs === undefined + ? {} + : { downloadTimeoutMs: options.downloadTimeoutMs }), filesystem: options.filesystem, idGenerator: options.idGenerator, + onDiagnostic: emitComponentInstallDiagnostic(options.eventBus, "ios"), processRunner: options.processRunner, }), ); @@ -197,11 +244,17 @@ export async function discoverDrivers(options: DriverDiscoveryContext): Promise< try { drivers.push( await AndroidDriver.create({ + acceptAndroidLicenses: options.acceptAndroidLicenses ?? false, clock: options.clock, + diskSpaceGuard, + ...(options.downloadTimeoutMs === undefined + ? {} + : { downloadTimeoutMs: options.downloadTimeoutMs }), env: process.env, filesystem: options.filesystem, homeDirectory: homedir(), idGenerator: options.idGenerator, + onDiagnostic: bridgeAndroidDriverDiagnostic(options.eventBus), processRunner: options.processRunner, }), ); @@ -251,6 +304,115 @@ async function loadDriversModule( return drivers; } +/** + * Turns a driver's `component-install-*` diagnostic into the matching `component.install-*` + * bus event. Drivers never depend on the event bus directly (architecture rule 5 -- loose + * coupling via the bus is for observers only) -- this is the one place, at driver construction, + * that bridges the driver's diagnostic callback to a post-commit fact for observers (`simlock + * events`, and the durable-log subscription in `startDaemon`). + */ +export function emitComponentInstallDiagnostic( + eventBus: Pick, + platform: "android" | "ios", +): (diagnostic: ComponentInstallDiagnostic) => void { + return (diagnostic) => { + switch (diagnostic.kind) { + case "component-install-started": + eventBus.emit( + "component.install-started", + { + componentId: diagnostic.componentId, + platform, + ...(diagnostic.requesterId === undefined + ? {} + : { requesterId: diagnostic.requesterId }), + }, + "driver-diagnostics", + ); + return; + case "component-installed": + eventBus.emit( + "component.installed", + { + componentId: diagnostic.componentId, + durationMs: diagnostic.durationMs, + platform, + ...(diagnostic.requesterId === undefined + ? {} + : { requesterId: diagnostic.requesterId }), + }, + "driver-diagnostics", + ); + return; + case "component-install-failed": + eventBus.emit( + "component.install-failed", + { + componentId: diagnostic.componentId, + durationMs: diagnostic.durationMs, + error: diagnostic.error, + platform, + ...(diagnostic.requesterId === undefined + ? {} + : { requesterId: diagnostic.requesterId }), + }, + "driver-diagnostics", + ); + return; + } + }; +} + +function isComponentInstallDiagnostic(diagnostic: { + readonly kind: string; +}): diagnostic is ComponentInstallDiagnostic { + return ( + diagnostic.kind === "component-install-started" || + diagnostic.kind === "component-installed" || + diagnostic.kind === "component-install-failed" + ); +} + +/** + * The Android driver's `onDiagnostic` also carries `snapshot-cold-boot` and + * `device-profile-source-unreadable` facts, neither wired to the bus (unchanged from before + * this change -- discovery never passed `onDiagnostic` to the Android driver at all, so every + * diagnostic it ever reported was already dropped). Only `component-install-*` is bridged here. + */ +export function bridgeAndroidDriverDiagnostic( + eventBus: Pick, +): (diagnostic: AndroidDriverDiagnostic) => void { + const installBridge = emitComponentInstallDiagnostic(eventBus, "android"); + return (diagnostic) => { + if (isComponentInstallDiagnostic(diagnostic)) { + installBridge(diagnostic); + } + }; +} + +/** + * Durable bookkeeping for component installs: the event ring buffer (`simlock events`) resets + * on daemon restart, so a component simlock installed on an agent's behalf is only attributable + * later through this log line -- see the `Logger` port ("Operational logging is a separate + * concern from the event bus" in ARCHITECTURE.md). + */ +export function wireComponentInstallLogging( + eventBus: Pick, + logger: Logger, +): void { + const componentsLogger = logger.child("components"); + eventBus.subscribe("component.installed", (envelope) => { + componentsLogger.info("Component installed", { + componentId: envelope.payload.componentId, + durationMs: envelope.payload.durationMs, + platform: envelope.payload.platform, + ...(envelope.payload.requesterId === undefined + ? {} + : { requesterId: envelope.payload.requesterId }), + }); + }); +} + /** * Best-effort logger for the fatal startup handler below. It cannot depend on the * daemon's own `Config` — that is exactly what may have failed to load — so it always diff --git a/src/daemon/server.test.ts b/src/daemon/server.test.ts index dc823b3..51c3a30 100644 --- a/src/daemon/server.test.ts +++ b/src/daemon/server.test.ts @@ -5,7 +5,16 @@ import { Socket, connect } from "node:net"; import { afterEach, describe, expect, it } from "vitest"; import { EventBus } from "../bus/index.js"; -import { type Config, CleanupReaper, FakeDriver, LeaseEngine, Registry } from "../core/index.js"; +import { + type Config, + CleanupReaper, + FakeDriver, + InsufficientDiskSpaceError, + LeaseEngine, + Registry, + RuntimeMissingError, +} from "../core/index.js"; +import { AndroidLicenseNotAcceptedError } from "../drivers/android/index.js"; import { FakeClock, FakeSystemStats, @@ -1073,6 +1082,168 @@ describe("DaemonServer lease heartbeat", () => { }); }); +describe("DaemonServer download policy", () => { + it("grants download permission under the always policy without a per-request flag", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ availableOsVersions: [], clock, platform: "ios" }); + const harness = await createHarness({ clock, downloads: { policy: "always" }, driver }); + const client = await createClient(harness.socketPath); + await hello(client); + + const grant = await client.request("lease.request", { + mode: "held", + requesterId: "agent-1", + request: { model: "iPhone 16", osVersion: "26.5", platform: "ios" }, + }); + + expect(grant.ok).toBe(true); + expect(driver.calls.find((call) => call.operation === "resolveSpec")?.arguments[1]).toEqual({ + allowDownload: true, + requesterId: "agent-1", + }); + await client.close(); + }); + + it("withholds download permission under the never policy even when the request asks for it", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ availableOsVersions: [], clock, platform: "ios" }); + const harness = await createHarness({ clock, downloads: { policy: "never" }, driver }); + const client = await createClient(harness.socketPath); + await hello(client); + + const response = await client.request("lease.request", { + allowDownload: true, + mode: "held", + requesterId: "agent-1", + request: { model: "iPhone 16", osVersion: "26.5", platform: "ios" }, + }); + + expect(response.ok).toBe(false); + expect(response.error).toMatchObject({ code: "RUNTIME_MISSING" }); + expect(response.error?.message).toContain("downloads.policy"); + expect(driver.calls.find((call) => call.operation === "resolveSpec")?.arguments[1]).toEqual({ + allowDownload: false, + requesterId: "agent-1", + }); + await client.close(); + }); + + it("defers to the request's own flag under the default on-request policy", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ availableOsVersions: [], clock, platform: "ios" }); + const harness = await createHarness({ clock, driver }); + const client = await createClient(harness.socketPath); + await hello(client); + + // No allowDownload on the request and the default policy, so this fails exactly as it + // did before the policy existed -- and, unlike the never-policy case above, the message + // is not attributed to configuration, since nothing in config forced the outcome. + const response = await client.request("lease.request", { + mode: "held", + requesterId: "agent-1", + request: { model: "iPhone 16", osVersion: "26.5", platform: "ios" }, + }); + + expect(response.ok).toBe(false); + expect(response.error).toMatchObject({ code: "RUNTIME_MISSING" }); + expect(response.error?.message).not.toContain("downloads.policy"); + await client.close(); + }); + + it("attaches the download-policy suffix under the never policy even when the request itself never asked for a download", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ availableOsVersions: [], clock, platform: "ios" }); + const harness = await createHarness({ clock, downloads: { policy: "never" }, driver }); + const client = await createClient(harness.socketPath); + await hello(client); + + // No allowDownload on this request at all -- the driver's own message still suggests + // `--allow-download`, which under the never policy can never help, so the suffix must + // still attach as the correction. + const response = await client.request("lease.request", { + mode: "held", + requesterId: "agent-1", + request: { model: "iPhone 16", osVersion: "26.5", platform: "ios" }, + }); + + expect(response.ok).toBe(false); + expect(response.error).toMatchObject({ code: "RUNTIME_MISSING" }); + expect(response.error?.message).toContain("downloads.policy"); + await client.close(); + }); + + it("never attaches the download-policy suffix to an undownloadable RuntimeMissingError, even under the never policy", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ availableOsVersions: [], clock, platform: "ios" }); + // Stands in for a real out-of-range / unpaired-runtime error: no download could ever have + // fixed this request, so the policy is not what blocked it. + driver.failOn( + "resolveSpec", + 1, + new RuntimeMissingError("ios", "12.0", { downloadable: false }), + ); + const harness = await createHarness({ clock, downloads: { policy: "never" }, driver }); + const client = await createClient(harness.socketPath); + await hello(client); + + const response = await client.request("lease.request", { + allowDownload: true, + mode: "held", + requesterId: "agent-1", + request: { model: "iPhone 16", osVersion: "12.0", platform: "ios" }, + }); + + expect(response.ok).toBe(false); + expect(response.error).toMatchObject({ code: "RUNTIME_MISSING" }); + expect(response.error?.message).not.toContain("downloads.policy"); + await client.close(); + }); +}); + +describe("DaemonServer error code mapping", () => { + it("maps InsufficientDiskSpaceError to INSUFFICIENT_DISK_SPACE", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ availableOsVersions: [], clock, platform: "ios" }); + driver.failOn("resolveSpec", 1, new InsufficientDiskSpaceError("ios", 8 * 1024 ** 3, 0)); + const harness = await createHarness({ clock, downloads: { policy: "always" }, driver }); + const client = await createClient(harness.socketPath); + await hello(client); + + const response = await client.request("lease.request", { + mode: "held", + requesterId: "agent-1", + request: { model: "iPhone 16", osVersion: "26.5", platform: "ios" }, + }); + + expect(response.ok).toBe(false); + expect(response.error).toMatchObject({ code: "INSUFFICIENT_DISK_SPACE" }); + await client.close(); + }); + + it("maps LicenseNotAcceptedError to LICENSE_NOT_ACCEPTED", async () => { + const clock = new FakeClock(1_000); + const driver = new FakeDriver({ availableOsVersions: [], clock, platform: "android" }); + driver.failOn( + "resolveSpec", + 1, + new AndroidLicenseNotAcceptedError("system-images;android-35;google_apis;arm64-v8a"), + ); + const harness = await createHarness({ clock, downloads: { policy: "always" }, driver }); + const client = await createClient(harness.socketPath); + await hello(client); + + const response = await client.request("lease.request", { + mode: "held", + requesterId: "agent-1", + request: { model: "Pixel 8", osVersion: "35", platform: "android" }, + }); + + expect(response.ok).toBe(false); + expect(response.error).toMatchObject({ code: "LICENSE_NOT_ACCEPTED" }); + await client.close(); + }); +}); + // fallow-ignore-next-line complexity -- a test harness whose branches are all trivial optional-parameter defaulting. async function createHarness( options: { @@ -1084,6 +1255,7 @@ async function createHarness( readonly clock?: FakeClock; readonly converge?: () => Promise; readonly dispose?: () => void; + readonly downloads?: Partial; readonly driver?: FakeDriver; readonly logger?: Logger; readonly settle?: () => Promise; @@ -1115,7 +1287,7 @@ async function createHarness( ...(options.latencyMs === undefined ? {} : { latencyMs: options.latencyMs }), platform: "ios", }); - const config = testConfig(options.lease); + const config = testConfig(options.lease, options.downloads); const engine = new LeaseEngine({ clock, config, @@ -1260,7 +1432,10 @@ function sequence() { return { generate: () => `${next++}` }; } -function testConfig(leaseOverrides?: Partial): Config { +function testConfig( + leaseOverrides?: Partial, + downloadsOverrides?: Partial, +): Config { return { diskPressure: { freeBytesThreshold: 10 * gibibyte }, eventBuffer: { capacity: 100 }, @@ -1273,6 +1448,12 @@ function testConfig(leaseOverrides?: Partial): Config { stableObservations: 2, }, stalledTransition: { thresholdMultiplier: 3, minimumThresholdMs: 60_000 }, + downloads: { + policy: "on-request", + acceptAndroidLicenses: false, + timeoutMs: 1_200_000, + ...downloadsOverrides, + }, idle: { deleteAfterMs: 60_000, shutdownAfterMs: 10_000 }, lease: { detachedTtlMs: 60_000, diff --git a/src/daemon/server.ts b/src/daemon/server.ts index 6ddf38e..214a77f 100644 --- a/src/daemon/server.ts +++ b/src/daemon/server.ts @@ -5,6 +5,9 @@ import { type DeviceRequest, type LeaseProgress, type LeaseRecord, + effectiveAllowDownload, + InsufficientDiskSpaceError, + LicenseNotAcceptedError, NoCapacityError, NoDriverError, QueueTimeoutError, @@ -545,6 +548,11 @@ export class DaemonServer { }; const mode = payload.mode === "detached" ? "detached" : "held"; const requesterId = optionalString(payload, "requesterId") ?? this.options.defaultRequesterId; + // The request's own flag is only the input to the policy, not the final answer: `never` + // overrides an explicit `true` (and is what should show up in the eventual "runtime + // missing" message below), `always` grants it without the caller asking. + const requestedAllowDownload = optionalBoolean(payload, "allowDownload") ?? false; + const downloadsPolicy = this.options.config.downloads.policy; let progressSocket: IpcConnection | undefined = connection.socket; const disposeProgress = () => { progressSocket = undefined; @@ -554,7 +562,7 @@ export class DaemonServer { let grant; try { grant = await this.options.leases.request(request, { - allowDownload: optionalBoolean(payload, "allowDownload") ?? false, + allowDownload: effectiveAllowDownload(downloadsPolicy, requestedAllowDownload), mode, noWait: optionalBoolean(payload, "noWait") ?? false, onProgress: (progress) => { @@ -565,6 +573,26 @@ export class DaemonServer { requesterId, ...(typeof payload.timeoutMs === "number" ? { timeoutMs: payload.timeoutMs } : {}), }); + } catch (error: unknown) { + // The driver only ever sees the clamped-to-false permission, so its own + // RuntimeMissingError just says "missing" -- it has no way to know config is what's + // standing between this request and success. Recover that distinction here, the one place + // that saw both sides, rather than teaching the driver about config. Attach the suffix + // whenever the `never` policy is active, regardless of whether this particular request + // asked for a download: the driver's message suggests `--allow-download`, which under + // `never` can never help, so the suffix is the correction every caller needs to see, not + // just the ones that happened to ask. Still gated on `downloadable`: a request no download + // could ever have fixed (out of range, an installed-but-unpaired runtime, older than the + // download floor) must not be blamed on the download policy -- that policy was never what + // stood between this request and success. + if ( + downloadsPolicy === "never" && + error instanceof RuntimeMissingError && + error.downloadable + ) { + error.message = `${error.message} (downloads are disabled by configuration: downloads.policy is "never")`; + } + throw error; } finally { connection.progressDisposers.delete(disposeProgress); connection.progressRequesters.delete(requesterId); @@ -934,6 +962,12 @@ function errorCode(error: unknown): string { if (error instanceof UnknownModelError) { return "UNKNOWN_MODEL"; } + if (error instanceof InsufficientDiskSpaceError) { + return "INSUFFICIENT_DISK_SPACE"; + } + if (error instanceof LicenseNotAcceptedError) { + return "LICENSE_NOT_ACCEPTED"; + } if (error instanceof UnknownLeaseError) { return "UNKNOWN_LEASE"; } diff --git a/src/drivers/android/device-profile-source.test.ts b/src/drivers/android/device-profile-source.test.ts new file mode 100644 index 0000000..9f8ba02 --- /dev/null +++ b/src/drivers/android/device-profile-source.test.ts @@ -0,0 +1,309 @@ +import { describe, expect, it } from "vitest"; + +import { MemoryFilesystem, ScriptedProcessRunner } from "../../ports/index.js"; +import { + BuiltinDeviceProfileSource, + DeviceProfileRegistry, + parseDevicesXml, + UserDeviceProfileSource, + type DeviceProfileSourceDiagnostic, +} from "./device-profile-source.js"; + +const avdmanager = "/android-sdk/cmdline-tools/latest/bin/avdmanager"; +const pixelDevices = `Available devices:\nid: 0 or "pixel_8"\n Name: Pixel 8\n OEM : Google\n`; +const devicesXmlPath = "/home/simlock/.android/devices.xml"; + +describe("BuiltinDeviceProfileSource", () => { + it("resolves by name or id, case-insensitively", async () => { + const runner = new ScriptedProcessRunner([ + processResult(pixelDevices), + processResult(pixelDevices), + processResult(pixelDevices), + ]); + const source = new BuiltinDeviceProfileSource(avdmanager, runner); + + await expect(source.resolve("Pixel 8")).resolves.toEqual({ + avdmanagerId: "pixel_8", + kind: "builtin", + name: "Pixel 8", + }); + await expect(source.resolve("PIXEL_8")).resolves.toEqual({ + avdmanagerId: "pixel_8", + kind: "builtin", + name: "Pixel 8", + }); + await expect(source.resolve("Pixel Fold")).resolves.toBeUndefined(); + }); + + it("lists resolvable model names", async () => { + const runner = new ScriptedProcessRunner([processResult(pixelDevices)]); + const source = new BuiltinDeviceProfileSource(avdmanager, runner); + + await expect(source.listModels()).resolves.toEqual(["Pixel 8"]); + }); +}); + +describe("UserDeviceProfileSource", () => { + it("resolves a properties profile mapped from devices.xml hardware fields", async () => { + const filesystem = await filesystemWithDevicesXml(devicesXml()); + const source = new UserDeviceProfileSource(devicesXmlPath, filesystem); + + await expect(source.resolve("My Custom Phone")).resolves.toEqual({ + hardwareProperties: { + "hw.device.manufacturer": "Acme", + "hw.device.name": "My Custom Phone", + "hw.lcd.density": "420", + "hw.lcd.height": "2400", + "hw.lcd.width": "1080", + "hw.ramSize": "6144", + }, + kind: "properties", + name: "My Custom Phone", + }); + }); + + it("resolves nothing for a model it does not have", async () => { + const filesystem = await filesystemWithDevicesXml(devicesXml()); + const source = new UserDeviceProfileSource(devicesXmlPath, filesystem); + + await expect(source.resolve("Pixel 8")).resolves.toBeUndefined(); + }); + + it("treats an absent file as no profiles without a diagnostic", async () => { + const filesystem = new MemoryFilesystem(); + const diagnostics: DeviceProfileSourceDiagnostic[] = []; + const source = new UserDeviceProfileSource(devicesXmlPath, filesystem, (diagnostic) => + diagnostics.push(diagnostic), + ); + + await expect(source.listModels()).resolves.toEqual([]); + expect(diagnostics).toEqual([]); + }); + + it("reports malformed devices.xml as a diagnostic instead of throwing", async () => { + const filesystem = await filesystemWithDevicesXml("not even close to xml {{{"); + const diagnostics: DeviceProfileSourceDiagnostic[] = []; + const source = new UserDeviceProfileSource(devicesXmlPath, filesystem, (diagnostic) => + diagnostics.push(diagnostic), + ); + + await expect(source.listModels()).resolves.toEqual([]); + await expect(source.resolve("anything")).resolves.toBeUndefined(); + expect(diagnostics).toHaveLength(2); + expect(diagnostics[0]).toMatchObject({ + kind: "device-profile-source-unreadable", + path: devicesXmlPath, + }); + }); + + it("reports a devices.xml with a newline embedded in a device name as a diagnostic and produces no profile", async () => { + // Stands in for `Google\ndisk.dataPartition.path=/evil`: + // a value that would inject an arbitrary extra config.ini line once + // `AndroidDriver#applyHardwareProperties` merges it in. Embedded directly (not via an XML + // entity) since `extractText` only trims leading/trailing whitespace, not internal + // characters. + const filesystem = await filesystemWithDevicesXml( + '' + + "Evil\nPhone" + + "", + ); + const diagnostics: DeviceProfileSourceDiagnostic[] = []; + const source = new UserDeviceProfileSource(devicesXmlPath, filesystem, (diagnostic) => + diagnostics.push(diagnostic), + ); + + await expect(source.listModels()).resolves.toEqual([]); + await expect(source.resolve("Evil\nPhone")).resolves.toBeUndefined(); + expect(diagnostics).toHaveLength(2); + expect(diagnostics[0]).toMatchObject({ + kind: "device-profile-source-unreadable", + path: devicesXmlPath, + }); + }); + + it("treats a well-formed but empty devices.xml as legitimately profile-less", async () => { + const filesystem = await filesystemWithDevicesXml( + '', + ); + const diagnostics: DeviceProfileSourceDiagnostic[] = []; + const source = new UserDeviceProfileSource(devicesXmlPath, filesystem, (diagnostic) => + diagnostics.push(diagnostic), + ); + + await expect(source.listModels()).resolves.toEqual([]); + expect(diagnostics).toEqual([]); + }); +}); + +describe("parseDevicesXml", () => { + it("maps named density buckets and KiB ram to config.ini-shaped values", () => { + const xml = ` + + + Bucket Phone + + + normal + xxhdpi + + 1440 + 3040 + + + + 4194304 + + + + `; + + expect(parseDevicesXml(xml)).toEqual([ + { + hardwareProperties: { + "hw.device.name": "Bucket Phone", + "hw.lcd.density": "480", + "hw.lcd.height": "3040", + "hw.lcd.width": "1440", + "hw.ramSize": "4096", + }, + name: "Bucket Phone", + }, + ]); + }); + + it("skips a device with no name", () => { + const xml = ` + 2048 + `; + + expect(parseDevicesXml(xml)).toEqual([]); + }); + + it("rejects a device name containing an embedded line break or NUL byte", () => { + const withNewline = + '' + + "Evil\nPhone"; + expect(() => parseDevicesXml(withNewline)).toThrow(); + + const withNul = + '' + + "Evil\u0000Phone"; + expect(() => parseDevicesXml(withNul)).toThrow(); + }); + + it("rejects a manufacturer value containing an embedded line break, routing config.ini injection attempts through the same rejection as the name field", () => { + const xml = + '' + + "Pixel Knockoff" + + "Google\ndisk.dataPartition.path=/evil" + + ""; + + expect(() => parseDevicesXml(xml)).toThrow(); + }); + + it("returns no profiles for an empty file without throwing", () => { + expect(parseDevicesXml("")).toEqual([]); + expect(parseDevicesXml(" \n ")).toEqual([]); + }); + + it("throws for content with no recognizable root", () => { + expect(() => parseDevicesXml("")).toThrow(); + expect(() => parseDevicesXml("this is not xml")).toThrow(); + }); +}); + +describe("DeviceProfileRegistry", () => { + it("resolves the first source's profile when two sources both name the same model", async () => { + const runner = new ScriptedProcessRunner([processResult(pixelDevices)]); + const builtin = new BuiltinDeviceProfileSource(avdmanager, runner); + const filesystem = await filesystemWithDevicesXml( + devicesXml().replace("My Custom Phone", "Pixel 8"), + ); + const user = new UserDeviceProfileSource(devicesXmlPath, filesystem); + const registry = new DeviceProfileRegistry([builtin, user]); + + await expect(registry.resolve("Pixel 8")).resolves.toEqual({ + avdmanagerId: "pixel_8", + kind: "builtin", + name: "Pixel 8", + }); + }); + + it("falls through to a later source when the first has no match", async () => { + const runner = new ScriptedProcessRunner([processResult(pixelDevices)]); + const builtin = new BuiltinDeviceProfileSource(avdmanager, runner); + const filesystem = await filesystemWithDevicesXml(devicesXml()); + const user = new UserDeviceProfileSource(devicesXmlPath, filesystem); + const registry = new DeviceProfileRegistry([builtin, user]); + + await expect(registry.resolve("My Custom Phone")).resolves.toEqual({ + hardwareProperties: { + "hw.device.manufacturer": "Acme", + "hw.device.name": "My Custom Phone", + "hw.lcd.density": "420", + "hw.lcd.height": "2400", + "hw.lcd.width": "1080", + "hw.ramSize": "6144", + }, + kind: "properties", + name: "My Custom Phone", + }); + }); + + it("rejects an unresolvable model with UnknownModelError", async () => { + const runner = new ScriptedProcessRunner([processResult(pixelDevices)]); + const builtin = new BuiltinDeviceProfileSource(avdmanager, runner); + const registry = new DeviceProfileRegistry([builtin]); + + await expect(registry.resolve("Nope")).rejects.toMatchObject({ name: "UnknownModelError" }); + }); + + it("dedupes listModels by name, earliest source winning", async () => { + const runner = new ScriptedProcessRunner([processResult(pixelDevices)]); + const builtin = new BuiltinDeviceProfileSource(avdmanager, runner); + const filesystem = await filesystemWithDevicesXml( + devicesXml().replace("My Custom Phone", "pixel 8"), + ); + const user = new UserDeviceProfileSource(devicesXmlPath, filesystem); + const registry = new DeviceProfileRegistry([builtin, user]); + + await expect(registry.listModels()).resolves.toEqual(["Pixel 8"]); + }); +}); + +function devicesXml(): string { + return ` + + + My Custom Phone + Acme + + + normal + 420dpi + + 1080 + 2400 + + + + 6144 + + + + `; +} + +async function filesystemWithDevicesXml(contents: string): Promise { + const filesystem = new MemoryFilesystem(); + await filesystem.mkdirp("/home/simlock/.android"); + await filesystem.writeFileAtomic(devicesXmlPath, contents); + return filesystem; +} + +function processResult(stdout: string) { + return { + match: { args: ["list", "device"], command: avdmanager }, + result: { code: 0, stderr: "", stdout }, + }; +} diff --git a/src/drivers/android/device-profile-source.ts b/src/drivers/android/device-profile-source.ts new file mode 100644 index 0000000..4ae789b --- /dev/null +++ b/src/drivers/android/device-profile-source.ts @@ -0,0 +1,437 @@ +import { DriverCrashError, UnknownModelError } from "../../core/driver.js"; +import type { Filesystem, ProcessRunner } from "../../ports/index.js"; + +/** + * What `DeviceProfileSource#resolve` hands back for a model name. `builtin` is today's + * `avdmanager -d ` path; `properties` is a hardware descriptor applied to `config.ini` + * after `avdmanager create avd` (see `AndroidDriver#provision`) -- there is no `avdmanager` + * device id for it because it never came from `avdmanager list device`. `name` is the + * source's own canonical spelling of the model (case may differ from what the caller asked + * for), which `AndroidDriver` needs so `DeviceSpec.model` and cache lookups stay stable. + */ +export type ResolvedDeviceProfile = + | { readonly kind: "builtin"; readonly name: string; readonly avdmanagerId: string } + | { + readonly kind: "properties"; + readonly name: string; + readonly hardwareProperties: Readonly>; + }; + +/** + * Diagnostics a `DeviceProfileSource` can raise. Sources never throw out of `resolve` / + * `listModels` for a data problem (an absent, unreadable, or malformed file is not the + * caller's fault) -- this is the only channel for surfacing that something was ignored. + */ +export interface DeviceProfileSourceDiagnostic { + readonly kind: "device-profile-source-unreadable"; + readonly path: string; + readonly reason: string; +} + +/** + * A read-only place `AndroidDriver` can load device profiles from. Simlock never writes to + * any of these locations -- see safety rule 1 -- sources only load. Implementations must + * never throw for a missing or malformed backing store; report that through the + * `onDiagnostic` callback they were constructed with instead, and resolve to "nothing here." + */ +export interface DeviceProfileSource { + /** Resolvable model names this source can currently answer for. */ + listModels(): Promise; + /** `undefined` when this source has no profile for `model` -- never throws for a miss. */ + resolve(model: string): Promise; +} + +/** + * Ordered list of sources, first match wins. This is the whole extension point: a future + * community/network source is a new `DeviceProfileSource` implementation plus one more entry + * in the list a driver is constructed with -- nothing else in the driver changes. + */ +export class DeviceProfileRegistry { + readonly #sources: readonly DeviceProfileSource[]; + + constructor(sources: readonly DeviceProfileSource[]) { + this.#sources = sources; + } + + /** Dedupes by name, case-insensitively; the earliest source in the list wins a collision. */ + async listModels(): Promise { + const seen = new Map(); + for (const source of this.#sources) { + for (const name of await source.listModels()) { + const key = name.toLocaleLowerCase(); + if (!seen.has(key)) { + seen.set(key, name); + } + } + } + return [...seen.values()]; + } + + async resolve(model: string): Promise { + for (const source of this.#sources) { + const resolved = await source.resolve(model); + if (resolved !== undefined) { + return resolved; + } + } + throw new UnknownModelError("android", model); + } +} + +/** Today's behavior, refactored behind `DeviceProfileSource`: resolves against `avdmanager list device`. */ +export class BuiltinDeviceProfileSource implements DeviceProfileSource { + readonly #avdmanager: string; + readonly #processRunner: ProcessRunner; + + constructor(avdmanager: string, processRunner: ProcessRunner) { + this.#avdmanager = avdmanager; + this.#processRunner = processRunner; + } + + // fallow-ignore-next-line unused-class-member -- reached through the DeviceProfileSource port by DeviceProfileRegistry.listModels. + async listModels(): Promise { + return (await this.#profiles()).map((profile) => profile.name); + } + + async resolve(model: string): Promise { + const normalized = model.toLocaleLowerCase(); + const profile = (await this.#profiles()).find( + (candidate) => + candidate.name.toLocaleLowerCase() === normalized || + candidate.id.toLocaleLowerCase() === normalized, + ); + return profile === undefined + ? undefined + : { avdmanagerId: profile.id, kind: "builtin", name: profile.name }; + } + + async #profiles(): Promise { + const result = await this.#processRunner.run(this.#avdmanager, ["list", "device"]); + if (result.code !== 0) { + throw new DriverCrashError( + `${this.#avdmanager} list device failed: ${result.stderr || result.stdout}`, + ); + } + return parseAvdmanagerDeviceProfiles(result.stdout); + } +} + +interface AvdmanagerDeviceProfile { + readonly id: string; + readonly name: string; +} + +export function parseAvdmanagerDeviceProfiles(output: string): AvdmanagerDeviceProfile[] { + const profiles: AvdmanagerDeviceProfile[] = []; + let id: string | undefined; + for (const line of output.split(/\r?\n/)) { + const idMatch = /^id:\s*\d+\s+or\s+"([^"]+)"/.exec(line.trim()); + if (idMatch?.[1] !== undefined) { + id = idMatch[1]; + continue; + } + const nameMatch = /^Name:\s*(.+)$/.exec(line.trim()); + if (id !== undefined && nameMatch?.[1] !== undefined) { + profiles.push({ id, name: nameMatch[1] }); + id = undefined; + } + } + return profiles; +} + +/** + * Read-only parse of Android Studio's `~/.android/devices.xml` -- that file belongs to + * Android Studio; Simlock only ever reads it. Resolved profiles carry hardware properties + * (screen resolution, density, RAM) applied to the simlock-created AVD's `config.ini`, since + * there is no official profile store beyond `avdmanager`'s own built-ins to create the AVD + * from directly. + * + * Parsed without a runtime dependency: a minimal, tolerant tag extractor rather than a real + * XML parser (see `parseDevicesXml` below for exactly what it reads and what it deliberately + * ignores). + */ +export class UserDeviceProfileSource implements DeviceProfileSource { + readonly #filesystem: Filesystem; + readonly #onDiagnostic: ((diagnostic: DeviceProfileSourceDiagnostic) => void) | undefined; + readonly #path: string; + + constructor( + path: string, + filesystem: Filesystem, + onDiagnostic?: (diagnostic: DeviceProfileSourceDiagnostic) => void, + ) { + this.#filesystem = filesystem; + this.#onDiagnostic = onDiagnostic; + this.#path = path; + } + + async listModels(): Promise { + return (await this.#profiles()).map((profile) => profile.name); + } + + async resolve(model: string): Promise { + const normalized = model.toLocaleLowerCase(); + const profile = (await this.#profiles()).find( + (candidate) => candidate.name.toLocaleLowerCase() === normalized, + ); + return profile === undefined + ? undefined + : { hardwareProperties: profile.hardwareProperties, kind: "properties", name: profile.name }; + } + + async #profiles(): Promise { + if (!(await this.#filesystem.exists(this.#path))) { + // Android Studio never having run, or never having any custom profiles, is the common + // case, not a diagnostic-worthy one. + return []; + } + + let contents: string; + try { + contents = await this.#filesystem.readFile(this.#path); + } catch (error: unknown) { + this.#reportUnreadable(errorMessage(error)); + return []; + } + + try { + return parseDevicesXml(contents); + } catch (error: unknown) { + this.#reportUnreadable(errorMessage(error)); + return []; + } + } + + #reportUnreadable(reason: string): void { + this.#onDiagnostic?.({ kind: "device-profile-source-unreadable", path: this.#path, reason }); + } +} + +interface DevicesXmlProfile { + readonly name: string; + readonly hardwareProperties: Readonly>; +} + +/** + * Maps a `` element to the `config.ini` hardware properties `avdmanager -d` would + * otherwise have produced. Mapped: + * + * - `` -> `hw.device.name` + * - `` -> `hw.device.manufacturer` (when present) + * - `` / `` -> `hw.lcd.width` / `hw.lcd.height` + * - `` -> `hw.lcd.density` (bucket name or `dpi` -> numeric dpi) + * - `` -> `hw.ramSize` (converted to MiB, unit-suffix-free like avdmanager writes it) + * + * Deliberately skipped, because `config.ini`'s AVD-identity hash and the emulator's own + * defaults cover them well enough that mirroring them exactly is not worth the parsing + * surface: `` / `` / `` (physical form + * factor, not a rendering input), `` / `` (physical DPI, distinct from the + * rendering `pixel-density` bucket this already maps), ``, ``, + * ``, ``, ``, ``, ``, `` (the system image + * the caller picked already determines the ABI), ``, and the whole `` + * block (API-level / feature compatibility simlock resolves separately via the system image). + * A device with multiple `` hardware variants (e.g. a foldable's postures) is read + * from whichever `` block appears first in the file, matching `` / + * `` textually rather than per-state. + */ +// fallow-ignore-next-line complexity -- per-field extraction and rejection checks are one parse pass over one block. +export function parseDevicesXml(contents: string): readonly DevicesXmlProfile[] { + const trimmed = contents.trim(); + if (trimmed === "") { + return []; + } + if (!/<([\w.-]+:)?devices[\s/>]/i.test(trimmed)) { + throw new Error("devices.xml has no recognizable root element"); + } + + const profiles: DevicesXmlProfile[] = []; + for (const block of extractElements(trimmed, "device")) { + const rawName = extractText(block, "name"); + if (rawName === undefined || rawName === "") { + continue; + } + // A value containing CR, LF, or NUL is never a legitimate device name or property -- + // devices.xml is Android Studio's own file, but Simlock only ever reads it (safety rule 1), + // and every value here eventually flows into a `config.ini` line-merge + // (`AndroidDriver#applyHardwareProperties` -> `#mergeConfigIniLines`). A newline there would + // inject an arbitrary extra `config.ini` key. Thrown rather than silently skipped or + // sanitized: this routes through the same malformed-devices.xml diagnostic path the caller + // (`UserDeviceProfileSource#profiles`) already has for an unparseable file, so a poisoned + // value is surfaced rather than quietly dropped. + if (containsForbiddenCharacter(rawName)) { + throw new Error(`devices.xml device name contains an embedded line break or NUL byte`); + } + const name = unescapeXml(rawName); + const hardwareProperties: Record = { "hw.device.name": name }; + + const manufacturer = extractText(block, "manufacturer"); + if (manufacturer !== undefined && manufacturer !== "") { + if (containsForbiddenCharacter(manufacturer)) { + throw new Error( + `devices.xml manufacturer for device "${name}" contains an embedded line break or NUL byte`, + ); + } + hardwareProperties["hw.device.manufacturer"] = unescapeXml(manufacturer); + } + + applyScreenProperties(block, hardwareProperties); + applyRamProperty(block, hardwareProperties); + + // Defense in depth beyond the per-field checks above: every value about to leave this + // parser (including ones a future field addition might forget to check individually) must + // be a single line before it is handed to the driver -- the same invariant + // `AndroidDriver#mergeConfigIniLines` enforces again on its own side, independently. + if (Object.values(hardwareProperties).some(containsForbiddenCharacter)) { + throw new Error( + `devices.xml property for device "${name}" contains an embedded line break or NUL byte`, + ); + } + + profiles.push({ hardwareProperties, name }); + } + return profiles; +} + +/** CR, LF, or NUL -- see the rejection check at the top of the `` loop above. */ +function containsForbiddenCharacter(value: string): boolean { + // oxlint-disable-next-line no-control-regex -- NUL rejection is intentional, not an accidental control-character match. + return /[\r\n\u0000]/.test(value); +} + +function applyScreenProperties( + deviceBlock: string, + hardwareProperties: Record, +): void { + const screen = extractText(deviceBlock, "screen"); + if (screen === undefined) { + return; + } + + const dimensions = extractText(screen, "dimensions"); + if (dimensions !== undefined) { + const width = extractText(dimensions, "x-dimension"); + const height = extractText(dimensions, "y-dimension"); + if (width !== undefined && /^\d+$/.test(width)) { + hardwareProperties["hw.lcd.width"] = width; + } + if (height !== undefined && /^\d+$/.test(height)) { + hardwareProperties["hw.lcd.height"] = height; + } + } + + const density = extractText(screen, "pixel-density"); + if (density !== undefined) { + const dpi = densityToDpi(density); + if (dpi !== undefined) { + hardwareProperties["hw.lcd.density"] = String(dpi); + } + } +} + +function applyRamProperty(deviceBlock: string, hardwareProperties: Record): void { + const ram = extractElement(deviceBlock, "ram-size"); + if (ram === undefined) { + return; + } + const megabytes = ramSizeToMebibytes(ram.text, ram.attributes["unit"]); + if (megabytes !== undefined) { + hardwareProperties["hw.ramSize"] = String(megabytes); + } +} + +/** Android's standard density buckets, matching what `avdmanager`'s own built-ins resolve to. */ +const DENSITY_BUCKETS_DPI: Readonly> = { + ldpi: 120, + mdpi: 160, + tvdpi: 213, + hdpi: 240, + xhdpi: 320, + xxhdpi: 480, + xxxhdpi: 640, +}; + +function densityToDpi(raw: string): number | undefined { + const key = raw.trim().toLowerCase(); + const bucket = DENSITY_BUCKETS_DPI[key]; + if (bucket !== undefined) { + return bucket; + } + const numeric = /^(\d+)(dpi)?$/.exec(key); + return numeric?.[1] === undefined ? undefined : Number(numeric[1]); +} + +function ramSizeToMebibytes(text: string, unit: string | undefined): number | undefined { + const value = Number(text.trim()); + if (!Number.isFinite(value)) { + return undefined; + } + switch ((unit ?? "MiB").trim().toUpperCase()) { + case "KIB": + return Math.round(value / 1024); + case "GIB": + return Math.round(value * 1024); + case "TIB": + return Math.round(value * 1024 * 1024); + default: + return Math.round(value); + } +} + +/** All top-level `<(ns:)tag>...` blocks' inner content, in document order. */ +function extractElements(xml: string, tag: string): string[] { + const pattern = new RegExp( + `<(?:[\\w.-]+:)?${tag}(?:\\s[^>]*)?>([\\s\\S]*?)`, + "gi", + ); + const blocks: string[] = []; + for (const match of xml.matchAll(pattern)) { + if (match[1] !== undefined) { + blocks.push(match[1]); + } + } + return blocks; +} + +/** The first `<(ns:)tag>...` block's inner content, or undefined. */ +function extractText(xml: string, tag: string): string | undefined { + return extractElement(xml, tag)?.text; +} + +/** The first `<(ns:)tag attr="...">...` element's attributes and trimmed text. */ +function extractElement( + xml: string, + tag: string, +): { readonly attributes: Readonly>; readonly text: string } | undefined { + const pattern = new RegExp( + `<(?:[\\w.-]+:)?${tag}((?:\\s[^>]*)?)>([\\s\\S]*?)`, + "i", + ); + const match = pattern.exec(xml); + if (match === null) { + return undefined; + } + const attributes: Record = {}; + for (const attributeMatch of (match[1] ?? "").matchAll(/([\w:-]+)\s*=\s*"([^"]*)"/g)) { + const [, key, value] = attributeMatch; + if (key !== undefined && value !== undefined) { + attributes[key] = value; + } + } + return { attributes, text: (match[2] ?? "").trim() }; +} + +const XML_ENTITIES: Readonly> = { + "&": "&", + "'": "'", + ">": ">", + "<": "<", + """: '"', +}; + +function unescapeXml(value: string): string { + return value.replace(/&(?:amp|apos|gt|lt|quot);/g, (entity) => XML_ENTITIES[entity] ?? entity); +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/drivers/android/index.test.ts b/src/drivers/android/index.test.ts index b5b219e..5381ae2 100644 --- a/src/drivers/android/index.test.ts +++ b/src/drivers/android/index.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import type { Driver } from "../../core/driver.js"; +import { DiskSpaceGuard, InsufficientDiskSpaceError } from "../../core/index.js"; import { FakeClock, type Filesystem, @@ -8,10 +9,17 @@ import { NodeFilesystem, NodeProcessRunner, ScriptedProcessRunner, + type ProcessHandle, + type ProcessRunOptions, type ScriptedProcessExpectation, SystemClock, } from "../../ports/index.js"; -import { AndroidDriver, SdkMissingError } from "./index.js"; +import { + AndroidDriver, + AndroidLicenseNotAcceptedError, + SdkMissingError, + type AndroidDriverDiagnostic, +} from "./index.js"; const sdk = "/android-sdk"; const home = "/home/simlock"; @@ -95,14 +103,17 @@ describe("AndroidDriver", () => { it("fails for a missing image unless downloads are explicitly allowed", async () => { const filesystem = await androidFilesystem(); - const runner = new ScriptedProcessRunner([ - processResult(binaries.avdmanager, ["list", "device"], pixelDevices), - processResult(binaries.avdmanager, ["list", "device"], pixelDevices), - processResult(binaries.sdkmanager, [ - "--install", - "system-images;android-35;google_apis;arm64-v8a", - ]), - ]); + const runner = new InstallReflectingProcessRunner( + [ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ], + filesystem, + ); const driver = await createDriver(filesystem, runner); const request = { model: "Pixel 8", osVersion: "35", platform: "android" } as const; @@ -648,6 +659,44 @@ describe("AndroidDriver", () => { expect(config).toContain("hw.ramSize=2048"); }); + it("rethrows a non-missing-file config.ini read error from the mark write instead of clobbering it as empty", async () => { + const configPath = `${avdDirectory}/simlock_one.avd/config.ini`; + const originalConfig = "hw.ramSize=2048\n"; + const permissionError = Object.assign(new Error("EACCES: permission denied"), { + code: "EACCES", + }); + const failureFilesystem = new ReadFailureFilesystem(configPath, permissionError); + const filesystem = await androidFilesystem({ config: originalConfig }, failureFilesystem); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, [ + "create", + "avd", + "-n", + "simlock_one", + "-k", + /.+/, + "-d", + "pixel_8", + ]), + processResult(binaries.emulator, ["-version"], "Android emulator version 36.1.9"), + processResult(binaries.adb, ["devices"], "List of devices attached\n"), + ...baselineBuildExpectations({ launchArgs: ["-no-snapshot-load"] }), + markWriteExpectation("emulator-5554", "device-2"), + ]); + const driver = await createDriver(filesystem, runner, { ids: ["one"] }); + const spec = await driver.resolveSpec( + { model: "Pixel 8", osVersion: "34", platform: "android" }, + { allowDownload: false }, + ); + const device = await driver.provision(spec); + + await expect(driver.makeReady(device)).rejects.toBe(permissionError); + + failureFilesystem.armed = false; + await expect(filesystem.readFile(configPath)).resolves.toBe(originalConfig); + }); + it("rewrites the mark on reclaim's snapshot-restore success path", async () => { const harness = await provisionedHarness({ forBaselineReclaim: true }); await harness.driver.makeReady(harness.device); @@ -832,6 +881,706 @@ describe("AndroidDriver", () => { const device = reality.devices.find((candidate) => candidate.deviceId === "simlock_legacy"); expect(device?.mark).toBeUndefined(); }); + + describe("device-profile sources", () => { + it("resolves a devices.xml-only model and applies its properties to config.ini before the config hash is captured", async () => { + const filesystem = await androidFilesystem(); + await writeDevicesXml(filesystem, customDeviceXml("Custom A", 4096)); + // `avdmanager create avd` is scripted (not a real process), so it never creates the AVD + // directory the way it would for real -- seed it here the same way, since applying + // hardware properties to config.ini right after create relies on that directory existing. + await filesystem.mkdirp(`${avdDirectory}/simlock_one.avd`); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, [ + "create", + "avd", + "-n", + "simlock_one", + "-k", + /.+/, + "-d", + "pixel_8", + ]), + processResult(binaries.emulator, ["-version"], "Android emulator version 36.1.9"), + processResult(binaries.adb, ["devices"], "List of devices attached\n"), + ]); + const driver = await createDriver(filesystem, runner, { ids: ["one"] }); + + const spec = await driver.resolveSpec( + { model: "Custom A", osVersion: "34", platform: "android" }, + { allowDownload: false }, + ); + expect(spec).toEqual({ model: "Custom A", osVersion: "34", platform: "android" }); + + const device = await driver.provision(spec); + + const config = await filesystem.readFile(`${avdDirectory}/simlock_one.avd/config.ini`); + expect(config).toContain("hw.device.name=Custom A"); + expect(config).toContain("hw.ramSize=4096"); + // The avdmanager `-d` seed uses a built-in device only to skip the interactive prompt -- + // it must never leak into the resolved spec or the applied hardware properties. + expect(config).not.toContain("pixel_8"); + expect(device.driverData).toMatchObject({ avdName: "simlock_one" }); + }); + + it("captures differing hardware properties in the config hash, proving they land before it is computed", async () => { + const buildHarness = async (ramMiB: number) => { + const filesystem = await androidFilesystem(); + await writeDevicesXml(filesystem, customDeviceXml("Custom A", ramMiB)); + await filesystem.mkdirp(`${avdDirectory}/simlock_one.avd`); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, [ + "create", + "avd", + "-n", + "simlock_one", + "-k", + /.+/, + "-d", + "pixel_8", + ]), + processResult(binaries.emulator, ["-version"], "Android emulator version 36.1.9"), + processResult(binaries.adb, ["devices"], "List of devices attached\n"), + ]); + const driver = await createDriver(filesystem, runner, { ids: ["one"] }); + const spec = await driver.resolveSpec( + { model: "Custom A", osVersion: "34", platform: "android" }, + { allowDownload: false }, + ); + return driver.provision(spec); + }; + + const lowRam = await buildHarness(2048); + const highRam = await buildHarness(4096); + + expect((lowRam.driverData as { configHash: string }).configHash).not.toBe( + (highRam.driverData as { configHash: string }).configHash, + ); + }); + + it("shadows a devices.xml profile with a built-in one of the same name and never applies properties", async () => { + const filesystem = await androidFilesystem(); + // Same name as the built-in fixture's "Pixel 8" -- the built-in source is registered + // first, so it must win and the devices.xml properties must never be touched. + await writeDevicesXml(filesystem, customDeviceXml("Pixel 8", 4096)); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, [ + "create", + "avd", + "-n", + "simlock_one", + "-k", + /.+/, + "-d", + "pixel_8", + ]), + processResult(binaries.emulator, ["-version"], "Android emulator version 36.1.9"), + processResult(binaries.adb, ["devices"], "List of devices attached\n"), + ]); + const driver = await createDriver(filesystem, runner, { ids: ["one"] }); + + const spec = await driver.resolveSpec( + { model: "Pixel 8", osVersion: "34", platform: "android" }, + { allowDownload: false }, + ); + await driver.provision(spec); + + // Only one `avdmanager list device` call happened (asserted implicitly by the runner + // never receiving the unscripted second call a `properties`-profile seed lookup would + // require), and no properties were merged into config.ini -- it was never even written. + await expect(filesystem.exists(`${avdDirectory}/simlock_one.avd/config.ini`)).resolves.toBe( + false, + ); + }); + + it("rejects a hardware-property value with an embedded line break, defense in depth beyond the devices.xml parser", async () => { + // `UserDeviceProfileSource`/`parseDevicesXml` already reject this at the devices.xml parse + // boundary (see device-profile-source.test.ts), but `DeviceProfileSource` is a documented + // extension point (see the interface's own doc comment) -- a future or third-party source + // could hand the driver a multiline value directly, bypassing that parser entirely. This + // exercises `#mergeConfigIniLines`'s own independent guard by going around the parser with + // a custom source, standing in for `Google\ndisk.dataPartition.path=/evil + // `. + const filesystem = await androidFilesystem(); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, [ + "create", + "avd", + "-n", + "simlock_one", + "-k", + /.+/, + "-d", + "pixel_8", + ]), + ]); + const maliciousSource = { + async listModels() { + return ["Evil Phone"]; + }, + async resolve(model: string) { + if (model.toLocaleLowerCase() !== "evil phone") { + return undefined; + } + return { + hardwareProperties: { + "hw.device.manufacturer": "Acme\ndisk.dataPartition.path=/evil", + "hw.device.name": "Evil Phone", + }, + kind: "properties" as const, + name: "Evil Phone", + }; + }, + }; + const driver = await AndroidDriver.create({ + clock: new FakeClock(), + deviceProfileSources: [maliciousSource], + env: { ANDROID_HOME: sdk }, + filesystem, + homeDirectory: home, + hostAbi: "arm64-v8a", + idGenerator: { generate: () => "one" }, + processRunner: runner, + }); + + const spec = await driver.resolveSpec( + { model: "Evil Phone", osVersion: "34", platform: "android" }, + { allowDownload: false }, + ); + + await expect(driver.provision(spec)).rejects.toThrow(/line break/); + // The rejected merge must never have reached the filesystem at all. + await expect(filesystem.exists(`${avdDirectory}/simlock_one.avd/config.ini`)).resolves.toBe( + false, + ); + }); + + it("surfaces malformed devices.xml as a diagnostic and falls through to UnknownModelError, never throwing from the parse itself", async () => { + const filesystem = await androidFilesystem(); + await filesystem.mkdirp(`${home}/.android`); + await filesystem.writeFileAtomic(`${home}/.android/devices.xml`, "not xml at all {{{"); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + ]); + const diagnostics: AndroidDriverDiagnostic[] = []; + const driver = await createDriver(filesystem, runner, { + onDiagnostic: (diagnostic) => diagnostics.push(diagnostic), + }); + + await expect( + driver.resolveSpec( + { model: "Nonexistent Model", osVersion: "34", platform: "android" }, + { allowDownload: false }, + ), + ).rejects.toMatchObject({ name: "UnknownModelError" }); + + expect(diagnostics).toEqual([ + expect.objectContaining({ kind: "device-profile-source-unreadable" }), + ]); + }); + }); + + describe("Android SDK license handling", () => { + const licenseNotAcceptedOutput = + "Warning: License for package Android SDK Platform 35 not accepted.\n\n" + + "1 package(s) were skipped due to license issues. Please accept the license(s) and try " + + "again.\nTo resolve, run: sdkmanager --licenses\n"; + + it("fails naming downloads.acceptAndroidLicenses when licenses are unaccepted and the flag is off", async () => { + const filesystem = await androidFilesystem(); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + { + match: { + args: ["--install", "system-images;android-35;google_apis;arm64-v8a"], + command: binaries.sdkmanager, + }, + result: { code: 1, stderr: "", stdout: licenseNotAcceptedOutput }, + }, + ]); + const driver = await createDriver(filesystem, runner, { acceptAndroidLicenses: false }); + + const error = await driver + .resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(AndroidLicenseNotAcceptedError); + expect((error as Error).message).toContain("downloads.acceptAndroidLicenses"); + expect((error as Error).message).toContain("sdkmanager --licenses"); + }); + + it("recognizes the alternate 'licenses have not been accepted' sdkmanager phrasing, not just 'not accepted'", async () => { + // The two documented sdkmanager phrasings this driver's license detection claims to + // handle (see the comment on `hasUnacceptedLicense`): a per-package warning ("... not + // accepted.") and this one, an aggregate summary with "been" between "not" and "accepted". + const licensesHaveNotBeenAcceptedOutput = + "1 of 7 SDK package license(s) not accepted.\n" + + "Review licenses that have not been accepted (see above)\n" + + "The licenses have not been accepted.\n"; + const filesystem = await androidFilesystem(); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + { + match: { + args: ["--install", "system-images;android-35;google_apis;arm64-v8a"], + command: binaries.sdkmanager, + }, + result: { code: 1, stderr: "", stdout: licensesHaveNotBeenAcceptedOutput }, + }, + ]); + const driver = await createDriver(filesystem, runner, { acceptAndroidLicenses: false }); + + const error = await driver + .resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(AndroidLicenseNotAcceptedError); + }); + + it("accepts licenses through piped confirmation and retries the install once when the flag is on", async () => { + const filesystem = await androidFilesystem(); + const runner = new InstallReflectingProcessRunner( + [ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + { + match: { + args: ["--install", "system-images;android-35;google_apis;arm64-v8a"], + command: binaries.sdkmanager, + }, + result: { code: 1, stderr: "", stdout: licenseNotAcceptedOutput }, + }, + processResult(binaries.sdkmanager, ["--licenses"], "All licenses accepted.\n"), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ], + filesystem, + ); + const driver = await createDriver(filesystem, runner, { acceptAndroidLicenses: true }); + + await expect( + driver.resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ), + ).resolves.toEqual({ model: "Pixel 8", osVersion: "35", platform: "android" }); + + const licensesCall = runner.calls.find( + (call) => call.command === binaries.sdkmanager && call.args[0] === "--licenses", + ); + expect(licensesCall?.options.input).toBe("y\n".repeat(100)); + // Exactly one retry: install, licenses, install again -- never a second acceptance pass. + expect(runner.calls.filter((call) => call.args[0] === "--install")).toHaveLength(2); + }); + + it("still fails when the install is rejected again after accepting licenses", async () => { + const filesystem = await androidFilesystem(); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + { + match: { + args: ["--install", "system-images;android-35;google_apis;arm64-v8a"], + command: binaries.sdkmanager, + }, + result: { code: 1, stderr: "", stdout: licenseNotAcceptedOutput }, + }, + processResult(binaries.sdkmanager, ["--licenses"], "All licenses accepted.\n"), + { + match: { + args: ["--install", "system-images;android-35;google_apis;arm64-v8a"], + command: binaries.sdkmanager, + }, + result: { code: 1, stderr: "still refusing", stdout: "" }, + }, + ]); + const driver = await createDriver(filesystem, runner, { acceptAndroidLicenses: true }); + + await expect( + driver.resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ), + ).rejects.toMatchObject({ name: "DriverCrashError" }); + }); + }); + + it("dedupes concurrent resolveSpec calls for the same missing system image behind one sdkmanager install", async () => { + const filesystem = await androidFilesystem(); + const runner = new InstallReflectingProcessRunner( + [ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ], + filesystem, + ); + const driver = await createDriver(filesystem, runner); + const request = { model: "Pixel 8", osVersion: "35", platform: "android" } as const; + + const [first, second] = await Promise.all([ + driver.resolveSpec(request, { allowDownload: true }), + driver.resolveSpec(request, { allowDownload: true }), + ]); + + expect(first).toEqual({ model: "Pixel 8", osVersion: "35", platform: "android" }); + expect(second).toEqual({ model: "Pixel 8", osVersion: "35", platform: "android" }); + expect(runner.calls.filter((call) => call.args[0] === "--install")).toHaveLength(1); + }); + + describe("component install diagnostics", () => { + it("reports component-install-started then component-installed with a duration on a clean install", async () => { + const filesystem = await androidFilesystem(); + const runner = new InstallReflectingProcessRunner( + [ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ], + filesystem, + ); + const diagnostics: AndroidDriverDiagnostic[] = []; + const driver = await createDriver(filesystem, runner, { + onDiagnostic: (diagnostic) => diagnostics.push(diagnostic), + }); + + await driver.resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ); + + expect(diagnostics).toEqual([ + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + kind: "component-install-started", + }, + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + durationMs: 0, + kind: "component-installed", + }, + ]); + }); + + it("reports component-install-failed with a stable error summary when the install is rejected outright", async () => { + const filesystem = await androidFilesystem(); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + { + match: { + args: ["--install", "system-images;android-35;google_apis;arm64-v8a"], + command: binaries.sdkmanager, + }, + result: { code: 1, stderr: "no network", stdout: "" }, + }, + ]); + const diagnostics: AndroidDriverDiagnostic[] = []; + const driver = await createDriver(filesystem, runner, { + onDiagnostic: (diagnostic) => diagnostics.push(diagnostic), + }); + + await driver + .resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ) + .catch((error: unknown) => error); + + expect(diagnostics).toEqual([ + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + kind: "component-install-started", + }, + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + durationMs: 0, + error: expect.stringContaining("DriverCrashError:"), + kind: "component-install-failed", + }, + ]); + }); + + it("reports exactly one component-install-failed for a license-retry failure, not one per attempt", async () => { + const licenseNotAcceptedOutput = + "Warning: License for package Android SDK Platform 35 not accepted.\n\n" + + "1 package(s) were skipped due to license issues. Please accept the license(s) and try " + + "again.\nTo resolve, run: sdkmanager --licenses\n"; + const filesystem = await androidFilesystem(); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + { + match: { + args: ["--install", "system-images;android-35;google_apis;arm64-v8a"], + command: binaries.sdkmanager, + }, + result: { code: 1, stderr: "", stdout: licenseNotAcceptedOutput }, + }, + processResult(binaries.sdkmanager, ["--licenses"], "All licenses accepted.\n"), + { + match: { + args: ["--install", "system-images;android-35;google_apis;arm64-v8a"], + command: binaries.sdkmanager, + }, + result: { code: 1, stderr: "still refusing", stdout: "" }, + }, + ]); + const diagnostics: AndroidDriverDiagnostic[] = []; + const driver = await createDriver(filesystem, runner, { + acceptAndroidLicenses: true, + onDiagnostic: (diagnostic) => diagnostics.push(diagnostic), + }); + + await driver + .resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ) + .catch((error: unknown) => error); + + const installDiagnostics = diagnostics.filter((diagnostic) => + diagnostic.kind.startsWith("component-install"), + ); + expect(installDiagnostics).toEqual([ + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + kind: "component-install-started", + }, + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + durationMs: 0, + error: expect.stringContaining("DriverCrashError:"), + kind: "component-install-failed", + }, + ]); + }); + + it("fails disk preflight before ever invoking sdkmanager, and reports no diagnostic", async () => { + const filesystem = await androidFilesystem({ freeDiskBytes: 1024 }); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + ]); + const diagnostics: AndroidDriverDiagnostic[] = []; + const driver = await createDriver(filesystem, runner, { + onDiagnostic: (diagnostic) => diagnostics.push(diagnostic), + }); + + const error = await driver + .resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(InsufficientDiskSpaceError); + expect((error as Error).message).toMatch(/needs ~2\.0 GiB.*only 0\.0 GiB available/); + // The device-profile lookup ran (avdmanager list device), but no sdkmanager call at all. + expect(runner.calls.some((call) => call.command === binaries.sdkmanager)).toBe(false); + expect(diagnostics).toEqual([]); + }); + + it("checks disk space on the SDK's own volume, not the daemon's working directory", async () => { + class RecordingFilesystem extends MemoryFilesystem { + readonly diskFreePaths: string[] = []; + + override async diskFree(path: string): Promise { + this.diskFreePaths.push(path); + return super.diskFree(path); + } + } + const recordingFilesystem = new RecordingFilesystem(); + const filesystem = await androidFilesystem({}, recordingFilesystem); + const runner = new InstallReflectingProcessRunner( + [ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ], + filesystem, + ); + const driver = await createDriver(filesystem, runner); + + await driver.resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ); + + expect(recordingFilesystem.diskFreePaths).toEqual([sdk]); + }); + + it("reports component-install-failed, never component-installed, when sdkmanager exits 0 but the image never shows up", async () => { + const filesystem = await androidFilesystem(); + // Deliberately a plain ScriptedProcessRunner, not InstallReflectingProcessRunner: sdkmanager + // claims success, but nothing ever lands in the filesystem's system-images tree -- the + // "reported success but still not installed" case the post-install re-scan exists to catch. + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ]); + const diagnostics: AndroidDriverDiagnostic[] = []; + const driver = await createDriver(filesystem, runner, { + onDiagnostic: (diagnostic) => diagnostics.push(diagnostic), + }); + + const error = await driver + .resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toContain( + "sdkmanager reported success but system-images;android-35;google_apis;arm64-v8a is still not installed", + ); + expect(diagnostics).toEqual([ + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + kind: "component-install-started", + }, + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + durationMs: 0, + error: expect.stringContaining("still not installed"), + kind: "component-install-failed", + }, + ]); + }); + + it("carries requesterId through to component-install diagnostics when resolveSpec's caller knows one", async () => { + const filesystem = await androidFilesystem(); + const runner = new InstallReflectingProcessRunner( + [ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ], + filesystem, + ); + const diagnostics: AndroidDriverDiagnostic[] = []; + const driver = await createDriver(filesystem, runner, { + onDiagnostic: (diagnostic) => diagnostics.push(diagnostic), + }); + + await driver.resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true, requesterId: "agent-7" }, + ); + + expect(diagnostics).toEqual([ + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + kind: "component-install-started", + requesterId: "agent-7", + }, + { + componentId: "system-images;android-35;google_apis;arm64-v8a", + durationMs: 0, + kind: "component-installed", + requesterId: "agent-7", + }, + ]); + }); + + it("respects disk-space reservations already outstanding on a shared DiskSpaceGuard", async () => { + const filesystem = await androidFilesystem({ freeDiskBytes: 2.5 * 1024 ** 3 }); + const runner = new ScriptedProcessRunner([ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + ]); + const diskSpaceGuard = new DiskSpaceGuard(); + // Stands in for another driver's (or another install's) concurrent reservation against the + // same shared guard -- 2 of the 2.5 GiB free is already spoken for, leaving less than the + // 2 GiB `ANDROID_SYSTEM_IMAGE_MIN_FREE_BYTES` floor this install needs. + const releaseOther = await diskSpaceGuard.reserve(filesystem, "ios", 1.5 * 1024 ** 3, sdk); + const driver = await createDriver(filesystem, runner, { diskSpaceGuard }); + + const error = await driver + .resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(InsufficientDiskSpaceError); + expect(runner.calls.some((call) => call.command === binaries.sdkmanager)).toBe(false); + releaseOther(); + }); + }); + + describe("download timeout", () => { + it("threads the configured downloadTimeoutMs into the sdkmanager install call", async () => { + const filesystem = await androidFilesystem(); + const runner = new InstallReflectingProcessRunner( + [ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ], + filesystem, + ); + const driver = await createDriver(filesystem, runner, { downloadTimeoutMs: 42_000 }); + + await driver.resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ); + + const installCall = runner.calls.find( + (call) => call.command === binaries.sdkmanager && call.args[0] === "--install", + ); + expect(installCall?.options.timeoutMs).toBe(42_000); + }); + + it("defaults to the same 20-minute timeout as before this option existed", async () => { + const filesystem = await androidFilesystem(); + const runner = new InstallReflectingProcessRunner( + [ + processResult(binaries.avdmanager, ["list", "device"], pixelDevices), + processResult(binaries.sdkmanager, [ + "--install", + "system-images;android-35;google_apis;arm64-v8a", + ]), + ], + filesystem, + ); + const driver = await createDriver(filesystem, runner); + + await driver.resolveSpec( + { model: "Pixel 8", osVersion: "35", platform: "android" }, + { allowDownload: true }, + ); + + const installCall = runner.calls.find( + (call) => call.command === binaries.sdkmanager && call.args[0] === "--install", + ); + expect(installCall?.options.timeoutMs).toBe(20 * 60_000); + }); + }); }); const live = process.env.SIMLOCK_LIVE_ANDROID === "1" ? it : it.skip; @@ -977,14 +1726,25 @@ async function createDriver( filesystem: Filesystem, processRunner: ScriptedProcessRunner, options: { + readonly acceptAndroidLicenses?: boolean; readonly clock?: FakeClock; + readonly diskSpaceGuard?: DiskSpaceGuard; + readonly downloadTimeoutMs?: number; readonly ids?: readonly string[]; + readonly onDiagnostic?: (diagnostic: AndroidDriverDiagnostic) => void; readonly readinessTimeoutMs?: number; } = {}, ) { let nextId = 0; return AndroidDriver.create({ + ...(options.acceptAndroidLicenses === undefined + ? {} + : { acceptAndroidLicenses: options.acceptAndroidLicenses }), clock: options.clock ?? new FakeClock(), + ...(options.diskSpaceGuard === undefined ? {} : { diskSpaceGuard: options.diskSpaceGuard }), + ...(options.downloadTimeoutMs === undefined + ? {} + : { downloadTimeoutMs: options.downloadTimeoutMs }), env: { ANDROID_HOME: sdk }, filesystem, homeDirectory: home, @@ -992,6 +1752,7 @@ async function createDriver( idGenerator: { generate: () => options.ids?.[nextId++] ?? `device-${nextId}`, }, + ...(options.onDiagnostic === undefined ? {} : { onDiagnostic: options.onDiagnostic }), ...(options.readinessTimeoutMs === undefined ? {} : { readinessTimeoutMs: options.readinessTimeoutMs }), @@ -999,13 +1760,53 @@ async function createDriver( }); } +async function writeDevicesXml(filesystem: MemoryFilesystem, deviceBodies: string): Promise { + await filesystem.mkdirp(`${home}/.android`); + await filesystem.writeFileAtomic( + `${home}/.android/devices.xml`, + `${deviceBodies}`, + ); +} + +function customDeviceXml(name: string, ramMiB: number): string { + return ( + `${name}` + + `${ramMiB}` + ); +} + +/** + * Fails every `readFile` of `failingPath` with a non-ENOENT error while `armed`, so a test can + * assert a caller rethrows it instead of treating it as an absent file -- then disarm to read + * the path back and confirm nothing overwrote it in the meantime. + */ +class ReadFailureFilesystem extends MemoryFilesystem { + armed = true; + + constructor( + private readonly failingPath: string, + private readonly error: Error, + freeDiskBytes?: number, + ) { + super(freeDiskBytes); + } + + override async readFile(path: string): Promise { + if (this.armed && path === this.failingPath) { + throw this.error; + } + return super.readFile(path); + } +} + async function androidFilesystem( options: { readonly config?: string; + readonly freeDiskBytes?: number; readonly images?: readonly (readonly [string, string, string])[]; } = {}, + filesystem: MemoryFilesystem = new MemoryFilesystem(options.freeDiskBytes), ): Promise { - const filesystem = new MemoryFilesystem(); for (const binary of Object.values(binaries)) { await filesystem.mkdirp(binary.slice(0, binary.lastIndexOf("/"))); await filesystem.writeFileAtomic(binary, "binary"); @@ -1107,6 +1908,49 @@ function baselineBuildExpectations(options: { return expectations; } +/** + * A `ScriptedProcessRunner` that also mirrors what real `sdkmanager` does on disk: a + * successful (`code: 0`, no unaccepted-license text) `--install ` call creates the + * corresponding `system-images/android-//` directory in `filesystem`. The + * driver's post-install `#installSystemImageOnce` re-scan needs the filesystem to actually + * reflect the "install" the same way the iOS driver's fixtures script a second `simctl list` + * response after a download (see `listFixtureAfterDownload` in the iOS driver's test file) -- + * a bare `ScriptedProcessRunner` only scripts the process's own stdout/stderr/exit code, never + * a filesystem side effect, so a scripted mkdirp-free "success" would fail the re-scan. + */ +class InstallReflectingProcessRunner extends ScriptedProcessRunner { + readonly #filesystem: MemoryFilesystem; + + constructor(expectations: readonly ScriptedProcessExpectation[], filesystem: MemoryFilesystem) { + super(expectations); + this.#filesystem = filesystem; + } + + override spawn( + command: string, + args: readonly string[], + options: ProcessRunOptions = {}, + ): ProcessHandle { + const handle = super.spawn(command, args, options); + if (args[0] === "--install" && typeof args[1] === "string") { + const packageName = args[1]; + void handle.wait().then((result) => { + const combined = `${result.stdout}\n${result.stderr}`; + const licenseNotAccepted = + /licen[cs]e/i.test(combined) && /not (?:been )?accepted/i.test(combined); + if (result.code === 0 && !licenseNotAccepted) { + const match = /^system-images;android-(.+);(.+);(.+)$/.exec(packageName); + if (match !== null) { + const [, api, tag, abi] = match; + void this.#filesystem.mkdirp(`${sdk}/system-images/android-${api}/${tag}/${abi}`); + } + } + }); + } + return handle; + } +} + function processResult(command: string, args: readonly (string | RegExp)[], stdout = "") { return { match: { args, command }, diff --git a/src/drivers/android/index.ts b/src/drivers/android/index.ts index b6326e1..c4b13ee 100644 --- a/src/drivers/android/index.ts +++ b/src/drivers/android/index.ts @@ -2,39 +2,64 @@ import type { DeviceSpec } from "../../core/domain.js"; import { BootTimeoutError, type DeviceRequest, + DiskSpaceGuard, type Driver, type DriverCatalogEntry, type DriverDevice, DriverCrashError, type DriverEstimate, type DriverReality, + LicenseNotAcceptedError, type ObservedDevice, type ObservedMark, type ReclaimResult, RuntimeMissingError, - UnknownModelError, } from "../../core/driver.js"; -import type { - Clock, - Filesystem, - IdGenerator, - ProcessHandle, - ProcessResult, - ProcessRunner, +import { stableError } from "../../core/stable-error.js"; +import type { ComponentInstallDiagnostic } from "../diagnostics.js"; +import { + isMissingPathError, + type Clock, + type Filesystem, + type IdGenerator, + type ProcessHandle, + type ProcessResult, + type ProcessRunner, } from "../../ports/index.js"; import { isAndroidDriverData, type AndroidDriverData } from "./data.js"; +import { + BuiltinDeviceProfileSource, + DeviceProfileRegistry, + parseAvdmanagerDeviceProfiles, + UserDeviceProfileSource, + type DeviceProfileSource, + type DeviceProfileSourceDiagnostic, + type ResolvedDeviceProfile, +} from "./device-profile-source.js"; const DEFAULT_READINESS_TIMEOUT_MS = 180_000; const COLD_BOOT_ESTIMATE_MS = 31_000; const PORT_MAX = 5682; const PORT_MIN = 5554; const PORT_POLL_INTERVAL_MS = 2_000; -const SDK_DOWNLOAD_TIMEOUT_MS = 20 * 60_000; +// Mirrors `downloads.timeoutMs`'s config default (`src/core/config.ts`) -- used only when a +// caller constructs the driver directly without threading the configured value through (tests, +// `SIMLOCK_DRIVERS_MODULE`). See the iOS driver's `DEFAULT_DOWNLOAD_TIMEOUT_MS` for the same +// pattern. +const DEFAULT_DOWNLOAD_TIMEOUT_MS = 20 * 60_000; +// `sdkmanager --licenses` prompts once per outstanding license with a bare `y/N`. Answering +// more times than there are real licenses is harmless -- the extra `y`s land after the prompt +// loop has already exited and sdkmanager simply never reads them -- so this just needs to be +// comfortably above the largest real Android SDK license count rather than exact. +const LICENSE_ACCEPT_ANSWERS = 100; // A defense-in-depth bound on the wait that follows a SIGKILL: NodeProcessHandle#wait // already settles shortly after `exit`, but this keeps a pathologically slow reap // from ever turning a "we already killed it" cleanup into an unbounded await. const SIGKILL_REAP_TIMEOUT_MS = 5_000; const SNAPSHOT_BOOT_ESTIMATE_MS = 4_000; +// Conservative estimate for a system-image download+install -- checked before `sdkmanager +// --install` ever starts, so a full disk fails fast instead of filling up mid-download. +const ANDROID_SYSTEM_IMAGE_MIN_FREE_BYTES = 2 * 1024 ** 3; const PROVISION_ESTIMATE_MS = 1_000; // Measured on an M3 Pro against Pixel 8 / API 35: 2.4-5.1s over nine steady-state reclaims // (median 4.6s), and 3.7-5.7s with three running at once. A `snapshot` reclaim loads the clean @@ -57,7 +82,29 @@ const DURABLE_MARK_KEY = "simlock.mark"; const ERASABLE_MARK_PATH = "/data/local/tmp/simlock-mark.json"; export interface AndroidDriverOptions { + /** + * Explicit legal consent for Android SDK licenses (`downloads.acceptAndroidLicenses`), + * independent of the per-request download permission. Defaults to `false`: an install that + * fails on an unaccepted license fails outright rather than accepting it silently. + */ + readonly acceptAndroidLicenses?: boolean; readonly clock: Clock; + /** + * Ordered device-profile sources, first match wins (see `DeviceProfileRegistry`). Defaults + * to `[builtin, user]` -- `avdmanager list device` first, then a read-only parse of + * `~/.android/devices.xml`, so a name defined in both resolves to the built-in. + */ + readonly deviceProfileSources?: readonly DeviceProfileSource[]; + /** + * Disk-space preflight, shared with every other driver that installs components -- see the + * iOS driver's `IosSimctlDriverOptions.diskSpaceGuard` for why a bare `assertDiskSpace` call + * isn't enough on its own. Defaults to a private, driver-local guard when omitted (tests, + * `SIMLOCK_DRIVERS_MODULE`); production wiring (`src/daemon/main.ts`) passes one shared + * instance to every driver. + */ + readonly diskSpaceGuard?: DiskSpaceGuard; + /** Per-install timeout for `sdkmanager`; defaults to `downloads.timeoutMs`'s own default. */ + readonly downloadTimeoutMs?: number; readonly env: Readonly>; readonly filesystem: Filesystem; readonly homeDirectory: string; @@ -68,11 +115,10 @@ export interface AndroidDriverOptions { readonly readinessTimeoutMs?: number; } -export interface AndroidDriverDiagnostic { - readonly avdName: string; - readonly kind: "snapshot-cold-boot"; - readonly readyAfterMs: number; -} +export type AndroidDriverDiagnostic = + | { readonly avdName: string; readonly kind: "snapshot-cold-boot"; readonly readyAfterMs: number } + | DeviceProfileSourceDiagnostic + | ComponentInstallDiagnostic; export class SdkMissingError extends Error { constructor(readonly searchedPaths: readonly string[]) { @@ -81,6 +127,17 @@ export class SdkMissingError extends Error { } } +export class AndroidLicenseNotAcceptedError extends LicenseNotAcceptedError { + constructor(readonly packageName: string) { + super("android", packageName); + this.message = + `sdkmanager refused to install ${packageName}: an Android SDK license is not accepted. ` + + `Set "downloads.acceptAndroidLicenses": true in config to accept automatically, or run ` + + `\`sdkmanager --licenses\` manually.`; + this.name = "AndroidLicenseNotAcceptedError"; + } +} + interface AndroidSdkPaths { readonly adb: string; readonly avdmanager: string; @@ -105,31 +162,34 @@ interface SystemImage { readonly version: string; } -interface DeviceProfile { - readonly id: string; - readonly name: string; -} - const allocationsByRunner = new WeakMap(); export class AndroidDriver implements Driver { readonly platform = "android" as const; + readonly #acceptAndroidLicenses: boolean; readonly #clock: Clock; + readonly #deviceProfiles: DeviceProfileRegistry; readonly #devices = new Map(); + readonly #diskSpaceGuard: DiskSpaceGuard; + readonly #downloadTimeoutMs: number; readonly #filesystem: Filesystem; readonly #hostAbi: string; readonly #idGenerator: IdGenerator; + readonly #installLocks = new Map>(); readonly #locks = new Map>(); readonly #onDiagnostic: ((diagnostic: AndroidDriverDiagnostic) => void) | undefined; readonly #portAllocator: PortAllocator; readonly #processRunner: ProcessRunner; - readonly #profiles = new Map(); + readonly #resolvedProfiles = new Map(); readonly #readinessTimeoutMs: number; readonly #sdk: AndroidSdkPaths; readonly #avdDirectory: string; private constructor(options: AndroidDriverOptions, sdk: AndroidSdkPaths) { + this.#acceptAndroidLicenses = options.acceptAndroidLicenses ?? false; this.#clock = options.clock; + this.#diskSpaceGuard = options.diskSpaceGuard ?? new DiskSpaceGuard(); + this.#downloadTimeoutMs = options.downloadTimeoutMs ?? DEFAULT_DOWNLOAD_TIMEOUT_MS; this.#filesystem = options.filesystem; this.#hostAbi = options.hostAbi ?? hostAbiFor(process.arch); this.#idGenerator = options.idGenerator ?? new SequentialIdGenerator(); @@ -139,6 +199,9 @@ export class AndroidDriver implements Driver { this.#sdk = sdk; this.#avdDirectory = options.env.ANDROID_AVD_HOME ?? `${options.homeDirectory}/.android/avd`; this.#portAllocator = portAllocatorFor(options.processRunner, sdk.adb); + this.#deviceProfiles = new DeviceProfileRegistry( + options.deviceProfileSources ?? defaultDeviceProfileSources(options, sdk, this.#onDiagnostic), + ); } static async create(options: AndroidDriverOptions): Promise { @@ -152,13 +215,13 @@ export class AndroidDriver implements Driver { async resolveSpec( request: DeviceRequest, - options: { readonly allowDownload: boolean }, + options: { readonly allowDownload: boolean; readonly requesterId?: string }, ): Promise { if (request.platform !== this.platform) { throw new Error(`Android driver cannot resolve ${request.platform} requests`); } - const profile = await this.#resolveProfile(request.model); + const profile = await this.#deviceProfiles.resolve(request.model); const images = await this.#installedImages(); const apiLevel = request.osVersion ?? newestApiLevel(images); if (apiLevel === undefined) { @@ -171,12 +234,10 @@ export class AndroidDriver implements Driver { } const packageName = systemImagePackage(apiLevel, "google_apis", this.#hostAbi); - await this.#runOrThrow(this.#sdk.sdkmanager, ["--install", packageName], { - timeoutMs: SDK_DOWNLOAD_TIMEOUT_MS, - }); + await this.#installSystemImage(packageName, options.requesterId); } - this.#profiles.set(profile.name.toLocaleLowerCase(), profile); + this.#resolvedProfiles.set(profile.name.toLocaleLowerCase(), profile); return { model: profile.name, osVersion: apiLevel, platform: this.platform }; } @@ -187,6 +248,13 @@ export class AndroidDriver implements Driver { const avdName = `simlock_${this.#idGenerator.generate()}`; const packageName = systemImagePackage(image.apiLevel, image.tag, image.abi); + // A `builtin` profile already carries the `avdmanager` device id `-d` wants. A + // `properties` profile has none -- it never came from `avdmanager list device` -- so + // `avdmanager create avd` is seeded with *some* built-in device (only to skip the + // interactive "custom hardware profile?" prompt) and the profile's own properties then + // overwrite that seed's config.ini values below, before anything reads them. + const seedDeviceId = + profile.kind === "builtin" ? profile.avdmanagerId : await this.#defaultAvdmanagerDeviceId(); await this.#runOrThrow(this.#sdk.avdmanager, [ "create", "avd", @@ -195,9 +263,16 @@ export class AndroidDriver implements Driver { "-k", packageName, "-d", - profile.id, + seedDeviceId, ]); + if (profile.kind === "properties") { + // Must happen before `#configHash` below captures the driver's snapshot/config-hash + // baseline: applying it after would let the baseline settle on the seed device's + // hardware and then see a spurious drift on the very next boot. + await this.#applyHardwareProperties(avdName, profile.hardwareProperties); + } + const configHash = await this.#configHash(avdName, image); const port = await this.#portAllocator.allocate(); const driverData: AndroidDriverData = { @@ -480,13 +555,13 @@ export class AndroidDriver implements Driver { } async listCatalog(): Promise { - const [profiles, images] = await Promise.all([ - this.#listDeviceProfiles(), + const [models, images] = await Promise.all([ + this.#deviceProfiles.listModels(), this.#installedImages(), ]); return { defaultRuntime: newestApiLevel(images), - models: profiles.map((profile) => profile.name), + models: [...models], runtimes: [...new Set(images.map((image) => image.apiLevel))].sort(compareApiLevels), }; } @@ -508,27 +583,175 @@ export class AndroidDriver implements Driver { } } - async #profileFor(model: string): Promise { - return this.#profiles.get(model.toLocaleLowerCase()) ?? this.#resolveProfile(model); + async #profileFor(model: string): Promise { + return ( + this.#resolvedProfiles.get(model.toLocaleLowerCase()) ?? this.#deviceProfiles.resolve(model) + ); } - async #listDeviceProfiles(): Promise { + /** See the seed-device comment at its `provision` call site. */ + async #defaultAvdmanagerDeviceId(): Promise { const result = await this.#runOrThrow(this.#sdk.avdmanager, ["list", "device"]); - return parseDeviceProfiles(result.stdout); + const [first] = parseAvdmanagerDeviceProfiles(result.stdout); + if (first === undefined) { + throw new DriverCrashError( + `${this.#sdk.avdmanager} list device reported no built-in device profiles`, + ); + } + return first.id; + } + + /** Merges `properties` into the AVD's `config.ini` -- see `#mergeConfigIniLines`. */ + async #applyHardwareProperties( + avdName: string, + properties: Readonly>, + ): Promise { + await this.#mergeConfigIniLines(avdName, properties); } - async #resolveProfile(model: string): Promise { - const profiles = await this.#listDeviceProfiles(); - const normalized = model.toLocaleLowerCase(); - const profile = profiles.find( - (candidate) => - candidate.name.toLocaleLowerCase() === normalized || - candidate.id.toLocaleLowerCase() === normalized, + /** + * Dedupes concurrent installs of the same system-image package behind one in-flight promise + * -- mirrors the iOS driver's `#downloadLocks`, sized to a package instead of a whole + * `xcodebuild` invocation. The map entry is removed once the install settles (success or + * failure), so a later, non-concurrent call starts a fresh attempt rather than replaying a + * stale result. + */ + async #installSystemImage(packageName: string, requesterId: string | undefined): Promise { + const inFlight = this.#installLocks.get(packageName); + if (inFlight !== undefined) { + return inFlight; + } + + const promise = this.#installSystemImageOnce(packageName, requesterId).finally(() => { + if (this.#installLocks.get(packageName) === promise) { + this.#installLocks.delete(packageName); + } + }); + this.#installLocks.set(packageName, promise); + return promise; + } + + /** + * Disk preflight (via the shared `DiskSpaceGuard`, released once the install settles either + * way), then the actual `sdkmanager` install, wrapped with `component.install-*` diagnostics + * -- split from `#installSystemImageOrThrow` below so the license-retry branching stays its + * own single-responsibility function rather than growing this one's complexity. A preflight + * failure is reported before any diagnostic fires: no install was actually attempted, so + * there is nothing to report as started or failed. The try/catch means a caller sees exactly + * one `install-failed` regardless of which branch below throws, never one per attempt. + * + * `component-installed` is a verified fact, not "`sdkmanager` exited 0 (possibly after a + * license-accept retry)": once the install call itself succeeds, this re-scans + * `#installedImages` and only reports `component-installed` once the package actually + * installed is present there. Absent (a "reported success but nothing showed up" case) + * reports `component-install-failed` instead and throws, matching the iOS driver's + * post-download verification. + */ + // fallow-ignore-next-line complexity -- reservation, install, and post-install verification are one attempt with one exit per outcome. + async #installSystemImageOnce( + packageName: string, + requesterId: string | undefined, + ): Promise { + const release = await this.#diskSpaceGuard.reserve( + this.#filesystem, + this.platform, + ANDROID_SYSTEM_IMAGE_MIN_FREE_BYTES, + this.#sdk.root, ); - if (profile === undefined) { - throw new UnknownModelError(this.platform, model); + try { + this.#onDiagnostic?.({ + componentId: packageName, + kind: "component-install-started", + ...(requesterId === undefined ? {} : { requesterId }), + }); + const startedAt = this.#clock.now(); + try { + await this.#installSystemImageOrThrow(packageName); + } catch (error: unknown) { + this.#onDiagnostic?.({ + componentId: packageName, + durationMs: this.#clock.now() - startedAt, + error: stableError(error), + kind: "component-install-failed", + ...(requesterId === undefined ? {} : { requesterId }), + }); + throw error; + } + + const images = await this.#installedImages(); + if ( + !images.some( + (image) => systemImagePackage(image.apiLevel, image.tag, image.abi) === packageName, + ) + ) { + const message = `sdkmanager reported success but ${packageName} is still not installed`; + this.#onDiagnostic?.({ + componentId: packageName, + durationMs: this.#clock.now() - startedAt, + error: message, + kind: "component-install-failed", + ...(requesterId === undefined ? {} : { requesterId }), + }); + throw new DriverCrashError(message); + } + + this.#onDiagnostic?.({ + componentId: packageName, + durationMs: this.#clock.now() - startedAt, + kind: "component-installed", + ...(requesterId === undefined ? {} : { requesterId }), + }); + } finally { + release(); + } + } + + /** + * Installs a system image, accepting Android SDK licenses first when `sdkmanager` refuses + * on an unaccepted one and `acceptAndroidLicenses` allows it -- never otherwise: license + * consent is independent of, and never implied by, download permission. + */ + async #installSystemImageOrThrow(packageName: string): Promise { + const result = await this.#processRunner.run(this.#sdk.sdkmanager, ["--install", packageName], { + timeoutMs: this.#downloadTimeoutMs, + }); + if (result.code === 0 && !hasUnacceptedLicense(result)) { + return; + } + if (!hasUnacceptedLicense(result)) { + throw new DriverCrashError( + `${this.#sdk.sdkmanager} --install ${packageName} failed: ${result.stderr || result.stdout}`, + ); + } + if (!this.#acceptAndroidLicenses) { + throw new AndroidLicenseNotAcceptedError(packageName); + } + + await this.#acceptLicenses(); + + const retry = await this.#processRunner.run(this.#sdk.sdkmanager, ["--install", packageName], { + timeoutMs: this.#downloadTimeoutMs, + }); + if (retry.code !== 0 || hasUnacceptedLicense(retry)) { + throw new DriverCrashError( + `${this.#sdk.sdkmanager} --install ${packageName} still failed after accepting licenses: ` + + `${retry.stderr || retry.stdout}`, + ); + } + } + + async #acceptLicenses(): Promise { + const result = await this.#processRunner.run(this.#sdk.sdkmanager, ["--licenses"], { + // `sdkmanager --licenses` prompts once per outstanding license; answering more times + // than there are real licenses is harmless (see `LICENSE_ACCEPT_ANSWERS`). + input: "y\n".repeat(LICENSE_ACCEPT_ANSWERS), + timeoutMs: this.#downloadTimeoutMs, + }); + if (result.code !== 0) { + throw new DriverCrashError( + `${this.#sdk.sdkmanager} --licenses failed: ${result.stderr || result.stdout}`, + ); } - return profile; } async #installedImages(): Promise { @@ -633,20 +856,56 @@ export class AndroidDriver implements Driver { } async #writeDurableMark(avdName: string, token: string): Promise { + await this.#mergeConfigIniLines(avdName, { [DURABLE_MARK_KEY]: token }); + } + + /** + * Reads `avdName`'s `config.ini`, merges `entries` into it -- overwriting any key already + * present, appending the rest -- and writes it back atomically. Shared by + * `#applyHardwareProperties` and `#writeDurableMark`, the driver's two config.ini + * read-modify-write sites. A missing file (the AVD's config.ini not created yet) starts the + * merge from empty content; any other read failure is rethrown rather than treated as an + * empty file -- silently starting from "" on, say, an EACCES or EIO would write back only + * `entries` and clobber whatever config.ini already held. + * + * Defense in depth against a config.ini injection: `#applyHardwareProperties` calls this with + * values sourced from a device profile (`avdmanager list device`, or a parsed + * `~/.android/devices.xml` -- see `device-profile-source.ts`'s own line-break rejection at the + * parse boundary). A key or value containing a line break would let one logical property + * inject arbitrary extra `config.ini` lines once joined in -- rejected here unconditionally, + * independent of and in addition to that parse-time check, so this merge is never the only + * thing standing between untrusted input and config.ini. + */ + async #mergeConfigIniLines( + avdName: string, + entries: Readonly>, + ): Promise { + for (const [key, value] of Object.entries(entries)) { + if (containsLineBreak(key) || containsLineBreak(value)) { + throw new DriverCrashError( + `Refusing to merge config.ini entry with an embedded line break (key ${JSON.stringify(key)})`, + ); + } + } const path = this.#configIniPath(avdName); let contents: string; try { contents = await this.#filesystem.readFile(path); - } catch { + } catch (error: unknown) { + if (!isMissingPathError(error)) { + throw error; + } contents = ""; } const lines = contents === "" ? [] : contents.replace(/\r?\n$/, "").split(/\r?\n/); - const markLine = `${DURABLE_MARK_KEY}=${token}`; - const existingIndex = lines.findIndex((line) => line.startsWith(`${DURABLE_MARK_KEY}=`)); - if (existingIndex >= 0) { - lines[existingIndex] = markLine; - } else { - lines.push(markLine); + for (const [key, value] of Object.entries(entries)) { + const line = `${key}=${value}`; + const existingIndex = lines.findIndex((entry) => entry.startsWith(`${key}=`)); + if (existingIndex >= 0) { + lines[existingIndex] = line; + } else { + lines.push(line); + } } await this.#filesystem.writeFileAtomic(path, `${lines.join("\n")}\n`); } @@ -1038,24 +1297,6 @@ function compareCommandLineToolVersions(left: string, right: string): number { return left.localeCompare(right); } -function parseDeviceProfiles(output: string): DeviceProfile[] { - const profiles: DeviceProfile[] = []; - let id: string | undefined; - for (const line of output.split(/\r?\n/)) { - const idMatch = /^id:\s*\d+\s+or\s+"([^"]+)"/.exec(line.trim()); - if (idMatch?.[1] !== undefined) { - id = idMatch[1]; - continue; - } - const nameMatch = /^Name:\s*(.+)$/.exec(line.trim()); - if (id !== undefined && nameMatch?.[1] !== undefined) { - profiles.push({ id, name: nameMatch[1] }); - id = undefined; - } - } - return profiles; -} - function newestApiLevel(images: readonly SystemImage[]): string | undefined { return [...new Set(images.map((image) => image.apiLevel))].sort(compareApiLevels).at(-1); } @@ -1154,6 +1395,11 @@ function portsFromAdbDevices(output: string): number[] { .filter((port) => Number.isInteger(port)); } +/** See `#mergeConfigIniLines`'s defense-in-depth check. */ +function containsLineBreak(value: string): boolean { + return /[\r\n]/.test(value); +} + function stableHash(parts: readonly string[]): string { let hash = 0x811c9dc5; for (const character of parts.join("\u0000")) { @@ -1167,6 +1413,38 @@ function hostAbiFor(architecture: string): string { return architecture === "arm64" ? "arm64-v8a" : "x86_64"; } +/** + * `sdkmanager --install` reports an unaccepted license in its output rather than through a + * dedicated exit code, so this is a best-effort text match against sdkmanager's own wording + * (e.g. `Warning: License for package ... not accepted.` / + * `... licenses have not been accepted.`), checked across both streams since sdkmanager splits + * its output between them across versions. + */ +function hasUnacceptedLicense(result: ProcessResult): boolean { + const combined = `${result.stdout}\n${result.stderr}`; + // Covers both documented sdkmanager phrasings: "License for package ... not accepted." and + // "licenses have not been accepted." -- the latter has "been" between "not" and "accepted". + return /licen[cs]e/i.test(combined) && /not (?:been )?accepted/i.test(combined); +} + +/** + * `[builtin, user]`: `avdmanager list device` first, then a read-only parse of Android + * Studio's `~/.android/devices.xml`. `ANDROID_SDK_HOME` (not `ANDROID_AVD_HOME`, which only + * relocates created AVDs) is the historical env var Android tooling uses to relocate the whole + * `~/.android` directory, including `devices.xml`. + */ +function defaultDeviceProfileSources( + options: AndroidDriverOptions, + sdk: AndroidSdkPaths, + onDiagnostic: ((diagnostic: AndroidDriverDiagnostic) => void) | undefined, +): readonly DeviceProfileSource[] { + const devicesXmlPath = `${options.env.ANDROID_SDK_HOME ?? options.homeDirectory}/.android/devices.xml`; + return [ + new BuiltinDeviceProfileSource(sdk.avdmanager, options.processRunner), + new UserDeviceProfileSource(devicesXmlPath, options.filesystem, onDiagnostic), + ]; +} + function portAllocatorFor(processRunner: ProcessRunner, adb: string): PortAllocator { const existing = allocationsByRunner.get(processRunner); if (existing !== undefined) { diff --git a/src/drivers/diagnostics.ts b/src/drivers/diagnostics.ts new file mode 100644 index 0000000..4672cf9 --- /dev/null +++ b/src/drivers/diagnostics.ts @@ -0,0 +1,31 @@ +/** + * Diagnostic shape both drivers report through their own `onDiagnostic` callback when a + * platform component (an iOS runtime, an Android system image) installs. Deliberately neutral + * -- nothing here references simctl or avdmanager concepts (architecture rule 2), so both + * driver modules can depend on it without depending on each other. + * + * Drivers never touch the event bus directly (architecture rule 5: "loose coupling via the bus + * is for observers only" -- a driver is not an observer of its own facts). The daemon layer + * bridges this diagnostic to the `component.install-*` bus events at driver construction time + * -- see `emitComponentInstallDiagnostic` in `src/daemon/main.ts`. + */ +export type ComponentInstallDiagnostic = + | { + readonly kind: "component-install-started"; + readonly componentId: string; + /** The requester on whose behalf this install runs, when the resolution that triggered it knew one. */ + readonly requesterId?: string; + } + | { + readonly kind: "component-installed"; + readonly componentId: string; + readonly durationMs: number; + readonly requesterId?: string; + } + | { + readonly kind: "component-install-failed"; + readonly componentId: string; + readonly durationMs: number; + readonly error: string; + readonly requesterId?: string; + }; diff --git a/src/drivers/ios/fixtures/simctl-list-pairing.json b/src/drivers/ios/fixtures/simctl-list-pairing.json new file mode 100644 index 0000000..dcb6010 --- /dev/null +++ b/src/drivers/ios/fixtures/simctl-list-pairing.json @@ -0,0 +1,43 @@ +{ + "devicetypes": [ + { + "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-16", + "name": "iPhone 16", + "minRuntimeVersion": 917504, + "maxRuntimeVersion": 16777215 + }, + { + "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-Xs", + "name": "iPhone Xs", + "minRuntimeVersion": 786432, + "maxRuntimeVersion": 1181184 + }, + { + "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-7", + "name": "iPhone 7", + "minRuntimeVersion": 589824, + "maxRuntimeVersion": 983040 + } + ], + "runtimes": [ + { + "identifier": "com.apple.CoreSimulator.SimRuntime.iOS-18-4", + "name": "iOS 18.4", + "version": "18.4", + "isAvailable": true, + "supportedDeviceTypes": [ + { "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-16", "name": "iPhone 16" }, + { "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-Xs", "name": "iPhone Xs" } + ] + }, + { + "identifier": "com.apple.CoreSimulator.SimRuntime.iOS-26-5", + "name": "iOS 26.5", + "version": "26.5", + "isAvailable": true, + "supportedDeviceTypes": [ + { "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-16", "name": "iPhone 16" } + ] + } + ] +} diff --git a/src/drivers/ios/fixtures/simctl-list.json b/src/drivers/ios/fixtures/simctl-list.json index 858f568..f1c6287 100644 --- a/src/drivers/ios/fixtures/simctl-list.json +++ b/src/drivers/ios/fixtures/simctl-list.json @@ -2,15 +2,21 @@ "devicetypes": [ { "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-17-Pro", - "name": "iPhone 17 Pro" + "name": "iPhone 17 Pro", + "minRuntimeVersion": 0, + "maxRuntimeVersion": 16777215 }, { "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-16", - "name": "iPhone 16" + "name": "iPhone 16", + "minRuntimeVersion": 0, + "maxRuntimeVersion": 16777215 }, { "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-15-Pro", - "name": "iPhone 15 Pro" + "name": "iPhone 15 Pro", + "minRuntimeVersion": 0, + "maxRuntimeVersion": 16777215 } ], "runtimes": [ @@ -18,13 +24,35 @@ "identifier": "com.apple.CoreSimulator.SimRuntime.iOS-18-4", "name": "iOS 18.4", "version": "18.4", - "isAvailable": true + "isAvailable": true, + "supportedDeviceTypes": [ + { + "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-17-Pro", + "name": "iPhone 17 Pro" + }, + { "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-16", "name": "iPhone 16" }, + { + "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-15-Pro", + "name": "iPhone 15 Pro" + } + ] }, { "identifier": "com.apple.CoreSimulator.SimRuntime.iOS-26-5", "name": "iOS 26.5", "version": "26.5", - "isAvailable": true + "isAvailable": true, + "supportedDeviceTypes": [ + { + "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-17-Pro", + "name": "iPhone 17 Pro" + }, + { "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-16", "name": "iPhone 16" }, + { + "identifier": "com.apple.CoreSimulator.SimDeviceType.iPhone-15-Pro", + "name": "iPhone 15 Pro" + } + ] }, { "identifier": "com.apple.CoreSimulator.SimRuntime.watchOS-26-5", diff --git a/src/drivers/ios/index.test.ts b/src/drivers/ios/index.test.ts index 85b67f6..52588f5 100644 --- a/src/drivers/ios/index.test.ts +++ b/src/drivers/ios/index.test.ts @@ -4,7 +4,9 @@ import { describe, expect, it } from "vitest"; import { BootTimeoutError, + DiskSpaceGuard, DriverCrashError, + InsufficientDiskSpaceError, RuntimeMissingError, UnknownModelError, } from "../../core/index.js"; @@ -17,6 +19,7 @@ import { SystemClock, type Filesystem, } from "../../ports/index.js"; +import type { ComponentInstallDiagnostic } from "../diagnostics.js"; import { IosSimctlDriver } from "./index.js"; const listFixture = readFileSync(new URL("./fixtures/simctl-list.json", import.meta.url), "utf8"); @@ -24,6 +27,30 @@ const listDevicesFixture = readFileSync( new URL("./fixtures/simctl-list-devices.json", import.meta.url), "utf8", ); +// iPhone 16 pairs with both installed runtimes; iPhone Xs pairs only with the older one (iOS 26 +// dropped it, mirroring real Xcode 27) and its range caps out at iOS 18.6; iPhone 7 caps out at +// iOS 15.0, below the 16.0 auto-download floor. +const pairingFixture = readFileSync( + new URL("./fixtures/simctl-list-pairing.json", import.meta.url), + "utf8", +); +// Same catalog as `listFixture`, plus an iOS 18.6 runtime that has just finished downloading -- +// stands in for the re-scanned catalog `resolveSpec` reads after a successful `xcodebuild` call. +const listFixtureAfterDownload = JSON.stringify({ + devicetypes: (JSON.parse(listFixture) as { devicetypes: unknown }).devicetypes, + runtimes: [ + ...(JSON.parse(listFixture) as { runtimes: unknown[] }).runtimes, + { + identifier: "com.apple.CoreSimulator.SimRuntime.iOS-18-6", + isAvailable: true, + name: "iOS 18.6", + supportedDeviceTypes: [ + { identifier: "com.apple.CoreSimulator.SimDeviceType.iPhone-16", name: "iPhone 16" }, + ], + version: "18.6", + }, + ], +}); const listInvocation = { command: "xcrun", args: ["simctl", "list", "-j", "devicetypes", "runtimes"], @@ -86,12 +113,14 @@ describe("IosSimctlDriver", () => { ).resolves.toEqual(spec); }); - it("rejects an unknown model", async () => { + it("rejects an unknown model, pointing at a newer Xcode", async () => { const driver = createDriver(scriptedListRunner()); + const result = await driver + .resolveSpec({ model: "iPhone 99", platform: "ios" }, { allowDownload: false }) + .catch((error: unknown) => error); - await expect( - driver.resolveSpec({ model: "iPhone 99", platform: "ios" }, { allowDownload: false }), - ).rejects.toBeInstanceOf(UnknownModelError); + expect(result).toBeInstanceOf(UnknownModelError); + expect(result).toMatchObject({ message: expect.stringContaining("newer Xcode") }); }); it("rejects malformed simctl catalog JSON without trusting partial data", async () => { @@ -110,17 +139,579 @@ describe("IosSimctlDriver", () => { ).rejects.toBeInstanceOf(DriverCrashError); }); - it("rejects missing runtimes even when downloads are allowed", async () => { - const driver = createDriver(scriptedListRunner()); + it("rejects a missing runtime without downloading when downloads are not allowed, naming the fix", async () => { + const runner = scriptedListRunner(); + const driver = createDriver(runner); + const result = await driver + .resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: false }, + ) + .catch((error: unknown) => error); + + expect(result).toBeInstanceOf(RuntimeMissingError); + expect(result).toMatchObject({ + message: expect.stringMatching(/18\.6/), + }); + expect(result).toMatchObject({ + message: expect.stringMatching(/allow-download|downloads\.policy/), + }); + // Never attempted a download: only the initial catalog list call happened. + expect(runner.calls).toHaveLength(1); + }); + + it("rejects an out-of-range OS version before ever considering a download", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: pairingFixture } }, + ]); + const driver = createDriver(runner); + + const result = await driver + .resolveSpec( + { model: "iPhone Xs", osVersion: "26.5", platform: "ios" }, + { allowDownload: true }, + ) + .catch((error: unknown) => error); + + expect(result).toBeInstanceOf(RuntimeMissingError); + expect(result).toMatchObject({ + downloadable: false, + message: expect.stringContaining("iPhone Xs supports iOS 12.0-18.6"), + }); + // No xcodebuild (or any further simctl) call: out-of-range is checked before download logic. + expect(runner.calls).toHaveLength(1); + }); + + it("rejects an installed runtime whose supportedDeviceTypes omits the requested model, without ever calling simctl create", async () => { + const catalog = JSON.stringify({ + devicetypes: [ + { + identifier: "com.apple.CoreSimulator.SimDeviceType.iPhone-16", + maxRuntimeVersion: 16_777_215, + minRuntimeVersion: 0, + name: "iPhone 16", + }, + ], + runtimes: [ + { + identifier: "com.apple.CoreSimulator.SimRuntime.iOS-18-4", + isAvailable: true, + name: "iOS 18.4", + supportedDeviceTypes: [], + version: "18.4", + }, + ], + }); + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: catalog } }, + ]); + const driver = createDriver(runner); + + const result = await driver + .resolveSpec( + { model: "iPhone 16", osVersion: "18.4", platform: "ios" }, + { allowDownload: true }, + ) + .catch((error: unknown) => error); + + expect(result).toBeInstanceOf(RuntimeMissingError); + expect(result).toMatchObject({ + downloadable: false, + message: "iOS 18.4 is installed but does not support iPhone 16", + }); + // Installed and in range, but not paired: no download attempted, no simctl create. + expect(runner.calls).toHaveLength(1); + }); + + it("selects the newest installed runtime that actually pairs with the model, not the newest overall", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: pairingFixture } }, + ]); + const driver = createDriver(runner); + + // iOS 26.5 is newer and installed, but only iOS 18.4 still lists iPhone Xs as supported. + await expect( + driver.resolveSpec({ model: "iPhone Xs", platform: "ios" }, { allowDownload: false }), + ).resolves.toEqual({ model: "iPhone Xs", osVersion: "18.4", platform: "ios" }); + }); + + it("refuses to auto-download a runtime older than the iOS 16.0 floor", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: pairingFixture } }, + ]); + const driver = createDriver(runner); + + const result = await driver + .resolveSpec( + { model: "iPhone 7", osVersion: "13.0", platform: "ios" }, + { allowDownload: true }, + ) + .catch((error: unknown) => error); + + expect(result).toBeInstanceOf(RuntimeMissingError); + expect(result).toMatchObject({ downloadable: false, message: expect.stringContaining("16.0") }); + // Range check passed (13.0 is within iPhone 7's 9.0-15.0), but no xcodebuild call was made. + expect(runner.calls).toHaveLength(1); + }); + + it("downloads a missing in-range runtime via xcodebuild and re-scans the catalog", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { + match: { + command: "xcodebuild", + args: ["-downloadPlatform", "iOS", "-buildVersion", "18.6"], + }, + }, + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixtureAfterDownload } }, + ]); + const driver = createDriver(runner); + + await expect( + driver.resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: true }, + ), + ).resolves.toEqual({ model: "iPhone 16", osVersion: "18.6", platform: "ios" }); + expect(runner.calls.map((call) => call.command)).toEqual(["xcrun", "xcodebuild", "xcrun"]); + }); + + it("dedupes concurrent resolveSpec calls for the same missing runtime behind one xcodebuild invocation", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { + match: { + command: "xcodebuild", + args: ["-downloadPlatform", "iOS", "-buildVersion", "18.6"], + }, + }, + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixtureAfterDownload } }, + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixtureAfterDownload } }, + ]); + const driver = createDriver(runner); + const request = { model: "iPhone 16", osVersion: "18.6", platform: "ios" } as const; + + const [first, second] = await Promise.all([ + driver.resolveSpec(request, { allowDownload: true }), + driver.resolveSpec(request, { allowDownload: true }), + ]); + + expect(first).toEqual({ model: "iPhone 16", osVersion: "18.6", platform: "ios" }); + expect(second).toEqual({ model: "iPhone 16", osVersion: "18.6", platform: "ios" }); + expect(runner.calls.filter((call) => call.command === "xcodebuild")).toHaveLength(1); + }); + + it("rejects a freshly downloaded exact-version runtime that does not pair with the requested device type", async () => { + // Mirrors the already-installed pairing check (`rejects an installed runtime whose + // supportedDeviceTypes omits the requested model` above), but for a runtime that only shows + // up *after* the download -- the refreshed catalog's iOS 18.6 exists but pairs with nothing. + const unpairedAfterDownload = JSON.stringify({ + devicetypes: (JSON.parse(listFixture) as { devicetypes: unknown }).devicetypes, + runtimes: [ + ...(JSON.parse(listFixture) as { runtimes: unknown[] }).runtimes, + { + identifier: "com.apple.CoreSimulator.SimRuntime.iOS-18-6", + isAvailable: true, + name: "iOS 18.6", + supportedDeviceTypes: [], + version: "18.6", + }, + ], + }); + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { + match: { + command: "xcodebuild", + args: ["-downloadPlatform", "iOS", "-buildVersion", "18.6"], + }, + }, + { match: listInvocation, result: { code: 0, stderr: "", stdout: unpairedAfterDownload } }, + ]); + const driver = createDriver(runner); + const result = await driver .resolveSpec( - { model: "iPhone 16", osVersion: "27", platform: "ios" }, + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, { allowDownload: true }, ) .catch((error: unknown) => error); expect(result).toBeInstanceOf(RuntimeMissingError); - expect(result).toMatchObject({ message: expect.stringContaining("install it via Xcode") }); + expect(result).toMatchObject({ + downloadable: false, + message: expect.stringContaining("does not support iPhone 16"), + }); + // Downloaded, but never committed to a spec: no simctl create followed the failed pairing check. + expect(runner.calls.map((call) => call.command)).toEqual(["xcrun", "xcodebuild", "xcrun"]); + }); + + describe("empty runtime catalog", () => { + // Devicetypes come from the Xcode install itself and are never empty on a working + // toolchain (an empty list there still means malformed JSON), but a fresh Xcode with zero + // simulator runtimes installed is a normal starting state -- `parseCatalog` must let it + // through so `resolveSpec` can reach the download-latest path instead of failing before any + // resolution is attempted. + const emptyRuntimesCatalog = JSON.stringify({ + devicetypes: [ + { + identifier: "com.apple.CoreSimulator.SimDeviceType.iPhone-16", + maxRuntimeVersion: 16_777_215, + minRuntimeVersion: 0, + name: "iPhone 16", + }, + ], + runtimes: [], + }); + + it("resolves via the download path when allowDownload is true", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: emptyRuntimesCatalog } }, + { match: { command: "xcodebuild", args: ["-downloadPlatform", "iOS"] } }, + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + ]); + const driver = createDriver(runner); + + await expect( + driver.resolveSpec({ model: "iPhone 16", platform: "ios" }, { allowDownload: true }), + ).resolves.toEqual({ model: "iPhone 16", osVersion: "26.5", platform: "ios" }); + }); + + it("gives a clean RuntimeMissingError, not a parse-time DriverCrashError, when allowDownload is false", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: emptyRuntimesCatalog } }, + ]); + const driver = createDriver(runner); + + const result = await driver + .resolveSpec({ model: "iPhone 16", platform: "ios" }, { allowDownload: false }) + .catch((error: unknown) => error); + + expect(result).toBeInstanceOf(RuntimeMissingError); + expect(result).not.toBeInstanceOf(DriverCrashError); + }); + + it("lists an empty runtimes catalog with defaultRuntime undefined instead of throwing", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: emptyRuntimesCatalog } }, + ]); + const driver = createDriver(runner); + + await expect(driver.listCatalog()).resolves.toEqual({ + defaultRuntime: undefined, + models: ["iPhone 16"], + runtimes: [], + }); + }); + }); + + describe("component install diagnostics", () => { + it("reports component-install-started then component-installed with a duration on a successful download", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { + match: { + command: "xcodebuild", + args: ["-downloadPlatform", "iOS", "-buildVersion", "18.6"], + }, + }, + { + match: listInvocation, + result: { code: 0, stderr: "", stdout: listFixtureAfterDownload }, + }, + ]); + const clock = new FakeClock(); + const diagnostics: ComponentInstallDiagnostic[] = []; + const driver = createDriver(runner, clock, new MemoryFilesystem(), (diagnostic) => + diagnostics.push(diagnostic), + ); + + await driver.resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: true }, + ); + + expect(diagnostics).toEqual([ + { componentId: "18.6", kind: "component-install-started" }, + { componentId: "18.6", durationMs: 0, kind: "component-installed" }, + ]); + }); + + it("reports component-install-failed with a stable error summary when xcodebuild fails", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { + match: { + command: "xcodebuild", + args: ["-downloadPlatform", "iOS", "-buildVersion", "18.6"], + }, + result: { code: 1, stderr: "no network", stdout: "" }, + }, + ]); + const diagnostics: ComponentInstallDiagnostic[] = []; + const driver = createDriver(runner, new FakeClock(), new MemoryFilesystem(), (diagnostic) => + diagnostics.push(diagnostic), + ); + + await driver + .resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: true }, + ) + .catch((error: unknown) => error); + + expect(diagnostics).toEqual([ + { componentId: "18.6", kind: "component-install-started" }, + { + componentId: "18.6", + durationMs: 0, + error: expect.stringContaining("DriverCrashError:"), + kind: "component-install-failed", + }, + ]); + }); + + it('reports "latest" as the component id for an unbounded default-runtime download', async () => { + // A device type with no upper bound (maxRuntimeVersion unbounded) but no currently + // installed runtime lists it as supported -- forces the "no paired runtime, download + // latest" branch rather than the exact-version one the other tests exercise. + const unpairedCatalog = JSON.stringify({ + devicetypes: [ + { + identifier: "com.apple.CoreSimulator.SimDeviceType.iPhone-16", + maxRuntimeVersion: 16_777_215, + minRuntimeVersion: 917_504, + name: "iPhone 16", + }, + ], + runtimes: [ + { + identifier: "com.apple.CoreSimulator.SimRuntime.iOS-18-4", + isAvailable: true, + name: "iOS 18.4", + supportedDeviceTypes: [], + version: "18.4", + }, + ], + }); + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: unpairedCatalog } }, + { match: { command: "xcodebuild", args: ["-downloadPlatform", "iOS"] } }, + { match: listInvocation, result: { code: 0, stderr: "", stdout: unpairedCatalog } }, + ]); + const diagnostics: ComponentInstallDiagnostic[] = []; + const driver = createDriver(runner, new FakeClock(), new MemoryFilesystem(), (diagnostic) => + diagnostics.push(diagnostic), + ); + + await driver + .resolveSpec({ model: "iPhone 16", platform: "ios" }, { allowDownload: true }) + .catch(() => undefined); + + expect(diagnostics[0]).toEqual({ componentId: "latest", kind: "component-install-started" }); + }); + + it("fails disk preflight before ever invoking xcodebuild, and reports no diagnostic", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + ]); + const diagnostics: ComponentInstallDiagnostic[] = []; + const filesystem = new MemoryFilesystem(1024); + const driver = createDriver(runner, new FakeClock(), filesystem, (diagnostic) => + diagnostics.push(diagnostic), + ); + + const error = await driver + .resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: true }, + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(InsufficientDiskSpaceError); + expect((error as Error).message).toMatch(/needs ~8\.0 GiB.*only 0\.0 GiB available/); + // Only the initial catalog list happened: no xcodebuild invocation, no diagnostic. + expect(runner.calls.map((call) => call.command)).toEqual(["xcrun"]); + expect(diagnostics).toEqual([]); + }); + + it("surfaces a disk-preflight failure from the bounded-default download path as InsufficientDiskSpaceError, not DriverCrashError", async () => { + // A device type with a finite max (bounded, unlike the "latest" test above) and no + // installed runtime pairs with it -- forces the bounded-default download branch, whose + // catch previously wrapped every failure (including this one) in a DriverCrashError. The + // installed runtime below keeps the catalog non-empty (required to parse at all) without + // pairing with the requested model. + const catalog = JSON.stringify({ + devicetypes: [ + { + identifier: "com.apple.CoreSimulator.SimDeviceType.iPhone-Xs", + maxRuntimeVersion: 1_181_184, + minRuntimeVersion: 786_432, + name: "iPhone Xs", + }, + ], + runtimes: [ + { + identifier: "com.apple.CoreSimulator.SimRuntime.iOS-18-4", + isAvailable: true, + name: "iOS 18.4", + supportedDeviceTypes: [], + version: "18.4", + }, + ], + }); + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: catalog } }, + ]); + const filesystem = new MemoryFilesystem(1024); + const driver = createDriver(runner, new FakeClock(), filesystem); + + const error = await driver + .resolveSpec({ model: "iPhone Xs", platform: "ios" }, { allowDownload: true }) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(InsufficientDiskSpaceError); + // Only the initial catalog list happened: no xcodebuild invocation attempted. + expect(runner.calls.map((call) => call.command)).toEqual(["xcrun"]); + }); + + it("checks disk space on the configured CoreSimulator volume, not the daemon's own working directory", async () => { + class RecordingFilesystem extends MemoryFilesystem { + readonly diskFreePaths: string[] = []; + + override async diskFree(path: string): Promise { + this.diskFreePaths.push(path); + return super.diskFree(path); + } + } + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { + match: { + command: "xcodebuild", + args: ["-downloadPlatform", "iOS", "-buildVersion", "18.6"], + }, + }, + { + match: listInvocation, + result: { code: 0, stderr: "", stdout: listFixtureAfterDownload }, + }, + ]); + const filesystem = new RecordingFilesystem(); + const driver = new IosSimctlDriver({ + clock: new FakeClock(), + coreSimulatorRoot: "/Users/agent/Library/Developer/CoreSimulator", + filesystem, + idGenerator: { generate: () => "device-1" }, + processRunner: runner, + }); + + await driver.resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: true }, + ); + + expect(filesystem.diskFreePaths).toEqual(["/Users/agent/Library/Developer/CoreSimulator"]); + }); + + it("reports component-install-failed, never component-installed, when xcodebuild exits 0 but the runtime never shows up", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { + match: { + command: "xcodebuild", + args: ["-downloadPlatform", "iOS", "-buildVersion", "18.6"], + }, + }, + // Deliberately re-scans to the SAME catalog: xcodebuild claims success, but no iOS 18.6 + // runtime is present -- the "reported success but still not installed" case the + // post-download re-scan exists to catch. + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + ]); + const diagnostics: ComponentInstallDiagnostic[] = []; + const driver = createDriver(runner, new FakeClock(), new MemoryFilesystem(), (diagnostic) => + diagnostics.push(diagnostic), + ); + + const error = await driver + .resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: true }, + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(DriverCrashError); + expect((error as Error).message).toContain("iOS 18.6 is still not installed"); + expect(diagnostics).toEqual([ + { componentId: "18.6", kind: "component-install-started" }, + { + componentId: "18.6", + durationMs: 0, + error: expect.stringContaining("still not installed"), + kind: "component-install-failed", + }, + ]); + }); + + it("carries requesterId through to component-install diagnostics when resolveSpec's caller knows one", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + { + match: { + command: "xcodebuild", + args: ["-downloadPlatform", "iOS", "-buildVersion", "18.6"], + }, + }, + { + match: listInvocation, + result: { code: 0, stderr: "", stdout: listFixtureAfterDownload }, + }, + ]); + const diagnostics: ComponentInstallDiagnostic[] = []; + const driver = createDriver(runner, new FakeClock(), new MemoryFilesystem(), (diagnostic) => + diagnostics.push(diagnostic), + ); + + await driver.resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: true, requesterId: "agent-7" }, + ); + + expect(diagnostics).toEqual([ + { componentId: "18.6", kind: "component-install-started", requesterId: "agent-7" }, + { + componentId: "18.6", + durationMs: 0, + kind: "component-installed", + requesterId: "agent-7", + }, + ]); + }); + + it("respects disk-space reservations already outstanding on a shared DiskSpaceGuard", async () => { + const runner = new ScriptedProcessRunner([ + { match: listInvocation, result: { code: 0, stderr: "", stdout: listFixture } }, + ]); + const filesystem = new MemoryFilesystem(9 * 1024 ** 3); + const diskSpaceGuard = new DiskSpaceGuard(); + // Stands in for another driver's (or another install's) concurrent reservation against the + // same shared guard -- 2 of the 9 GiB free is already spoken for, leaving less than the + // 8 GiB `IOS_RUNTIME_MIN_FREE_BYTES` floor this download needs. + const releaseOther = await diskSpaceGuard.reserve(filesystem, "android", 2 * 1024 ** 3, "."); + const driver = createDriver(runner, new FakeClock(), filesystem, undefined, diskSpaceGuard); + + const error = await driver + .resolveSpec( + { model: "iPhone 16", osVersion: "18.6", platform: "ios" }, + { allowDownload: true }, + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(InsufficientDiskSpaceError); + expect(runner.calls.map((call) => call.command)).toEqual(["xcrun"]); + releaseOther(); + }); }); it("provisions with the exact simctl argv and returns opaque iOS driver data", async () => { @@ -578,11 +1169,15 @@ function createDriver( runner: ScriptedProcessRunner, clock = new FakeClock(), filesystem: Filesystem = new MemoryFilesystem(), + onDiagnostic?: (diagnostic: ComponentInstallDiagnostic) => void, + diskSpaceGuard?: DiskSpaceGuard, ): IosSimctlDriver { return new IosSimctlDriver({ clock, + ...(diskSpaceGuard === undefined ? {} : { diskSpaceGuard }), filesystem, idGenerator: { generate: () => "device-1" }, + ...(onDiagnostic === undefined ? {} : { onDiagnostic }), processRunner: runner, }); } diff --git a/src/drivers/ios/index.ts b/src/drivers/ios/index.ts index e277cc6..1daaf21 100644 --- a/src/drivers/ios/index.ts +++ b/src/drivers/ios/index.ts @@ -1,12 +1,14 @@ import { BootTimeoutError, type DeviceRequest, + DiskSpaceGuard, type Driver, type DriverCatalogEntry, type DriverDevice, DriverCrashError, type DriverEstimate, type DriverReality, + InsufficientDiskSpaceError, type ObservedDevice, type ObservedRunState, RuntimeMissingError, @@ -14,6 +16,7 @@ import { } from "../../core/index.js"; import type { ObservedMark } from "../../core/driver.js"; import type { DeviceSpec } from "../../core/index.js"; +import { stableError } from "../../core/stable-error.js"; import type { Clock, Filesystem, @@ -21,10 +24,25 @@ import type { ProcessResult, ProcessRunner, } from "../../ports/index.js"; +import type { ComponentInstallDiagnostic } from "../diagnostics.js"; const COMMAND_TIMEOUT_MS = 30_000; const BOOTSTATUS_TIMEOUT_MS = 120_000; const PROVISION_ESTIMATE_MS = 500; +// Mirrors `downloads.timeoutMs`'s config default (`src/core/config.ts`) -- used only when a +// caller constructs the driver directly without threading the configured value through (tests, +// `SIMLOCK_DRIVERS_MODULE`). +const DEFAULT_DOWNLOAD_TIMEOUT_MS = 1_200_000; +// `simctl`'s `minRuntimeVersion` / `maxRuntimeVersion` encode "no bound" as 0xFFFFFF +// (255.255.255) rather than omitting the field. +const UNBOUNDED_VERSION = 0xff_ff_ff; +// `xcodebuild -downloadPlatform iOS -buildVersion` only reaches back to iOS 16.0 (Xcode +// 16.1+); older runtimes must be installed through Xcode itself. +const IOS_DOWNLOAD_FLOOR: readonly [number, number, number] = [16, 0, 0]; +// Conservative estimate for a simulator runtime download+install (~7 GB observed, rounded up +// with headroom) -- checked before `xcodebuild -downloadPlatform` ever starts, so a full disk +// fails fast instead of filling up mid-download. +const IOS_RUNTIME_MIN_FREE_BYTES = 8 * 1024 ** 3; // A cold `simctl boot` to `bootstatus` measures roughly 30s on a fast, idle machine and up to // a minute on a loaded or slower one. The upper end is the estimate, deliberately: this number // is what a waiting requester is quoted, and quoting 30s to someone who then waits 60s is the @@ -46,14 +64,44 @@ interface IosDriverData { export interface IosSimctlDriverOptions { readonly clock: Clock; + /** + * Volume a runtime download actually lands on, for the disk preflight in + * `#installComponent` -- simulator runtimes install under `~/Library/Developer/ + * CoreSimulator`, which is not necessarily the same volume as the daemon's working + * directory. Defaults to `"."` (the daemon process's own volume) only when nothing better + * is available, mirroring the Android driver's use of `sdk.root`. + */ + readonly coreSimulatorRoot?: string; + /** Per-download timeout; defaults to `downloads.timeoutMs`'s own default. */ + readonly downloadTimeoutMs?: number; + /** + * Disk-space preflight, shared with every other driver that installs components -- a bare + * `assertDiskSpace` call only ever sees an instantaneous free-space reading, so two concurrent + * installs (this driver's and the Android driver's, or two of this driver's own) can each pass + * it and jointly overfill the volume neither alone would have. Defaults to a private, + * driver-local guard when omitted (tests, `SIMLOCK_DRIVERS_MODULE`); production wiring + * (`src/daemon/main.ts`) passes one instance to every driver so the tracking is actually + * shared. + */ + readonly diskSpaceGuard?: DiskSpaceGuard; readonly filesystem: Filesystem; readonly idGenerator: IdGenerator; + /** + * Reports `component.install-*` facts for the daemon layer to bridge onto the event bus -- + * this driver never depends on the bus directly (architecture rule 5). Mirrors the Android + * driver's `onDiagnostic` option. + */ + readonly onDiagnostic?: (diagnostic: ComponentInstallDiagnostic) => void; readonly processRunner: ProcessRunner; } interface DeviceType { readonly identifier: string; readonly name: string; + /** Decoded `0xAABBCC` -> `[AA, BB, CC]`; simctl's inclusive lower bound on pairable runtimes. */ + readonly minRuntimeVersion: number; + /** Same encoding; `UNBOUNDED_VERSION` means "no upper bound". */ + readonly maxRuntimeVersion: number; } interface Runtime { @@ -61,6 +109,8 @@ interface Runtime { readonly name: string; readonly version: string; readonly isAvailable: boolean; + /** Device type identifiers this runtime pairs with -- authoritative once the runtime is installed. */ + readonly supportedDeviceTypeIds: ReadonlySet; } interface SimctlCatalog { @@ -82,22 +132,31 @@ type ProcessOutcome = export class IosSimctlDriver implements Driver { readonly platform = "ios" as const; readonly #clock: Clock; + readonly #coreSimulatorRoot: string; + readonly #diskSpaceGuard: DiskSpaceGuard; + readonly #downloadLocks = new Map>(); + readonly #downloadTimeoutMs: number; readonly #filesystem: Filesystem; readonly #idGenerator: IdGenerator; + readonly #onDiagnostic: ((diagnostic: ComponentInstallDiagnostic) => void) | undefined; readonly #processRunner: ProcessRunner; readonly #resolvedSpecs = new Map(); #devicesRoot: string | undefined; constructor(options: IosSimctlDriverOptions) { this.#clock = options.clock; + this.#coreSimulatorRoot = options.coreSimulatorRoot ?? "."; + this.#diskSpaceGuard = options.diskSpaceGuard ?? new DiskSpaceGuard(); + this.#downloadTimeoutMs = options.downloadTimeoutMs ?? DEFAULT_DOWNLOAD_TIMEOUT_MS; this.#filesystem = options.filesystem; this.#idGenerator = options.idGenerator; + this.#onDiagnostic = options.onDiagnostic; this.#processRunner = options.processRunner; } async resolveSpec( request: DeviceRequest, - _options: { readonly allowDownload: boolean }, + options: { readonly allowDownload: boolean; readonly requesterId?: string }, ): Promise { this.#requireIosPlatform(request.platform); const catalog = await this.#loadCatalog(); @@ -106,19 +165,194 @@ export class IosSimctlDriver implements Driver { ); if (deviceType === undefined) { - throw new UnknownModelError(this.platform, request.model); + throw new IosUnknownModelError(request.model); } - const installedRuntimes = catalog.runtimes.filter((runtime) => runtime.isAvailable); - const runtime = - request.osVersion === undefined - ? newestRuntime(installedRuntimes) - : installedRuntimes.find((candidate) => candidate.version === request.osVersion); + return request.osVersion === undefined + ? this.#resolveDefaultRuntime(deviceType, catalog, options) + : this.#resolveExactRuntime(deviceType, request.osVersion, catalog, options); + } + + /** + * Version requested explicitly: validated against the model's `[min, max]` pairing range + * *before* anything else -- an out-of-range request can never be fixed by downloading, so it + * must never even reach the download decision. + */ + // fallow-ignore-next-line complexity -- range/pairing checks, the download decision, and post-download verification are one resolution attempt. + async #resolveExactRuntime( + deviceType: DeviceType, + osVersion: string, + catalog: SimctlCatalog, + options: { readonly allowDownload: boolean; readonly requesterId?: string }, + ): Promise { + if (!isVersionInRange(osVersion, deviceType)) { + throw new IosVersionOutOfRangeError(deviceType.name, osVersion, deviceType); + } + + const installed = findInstalledRuntime(catalog, osVersion); + if (installed !== undefined) { + // In the model's declared `[min, max]` range is necessary but not sufficient: a runtime + // can be installed and still not pair with this specific device type (`supportedDeviceTypeIds` + // is the authoritative source once a runtime is actually on disk -- the range above is + // only a static hint). Checked before committing, so a mismatch never reaches `simctl create`. + if (!installed.supportedDeviceTypeIds.has(deviceType.identifier)) { + throw new IosRuntimeUnpairedError(deviceType.name, osVersion); + } + return this.#commitResolution(deviceType, installed); + } + if (!options.allowDownload) { + throw new IosRuntimeMissingError( + osVersion, + `iOS ${osVersion} is not installed; pass --allow-download (or set downloads.policy) ` + + `to download it`, + ); + } + + if (isTooOldToDownload(osVersion)) { + throw new IosDownloadFloorError(osVersion); + } + + const startedAt = await this.#downloadRuntime( + osVersion, + ["-downloadPlatform", "iOS", "-buildVersion", osVersion], + options.requesterId, + ); + // `component.installed` is a verified fact, not "xcodebuild exited 0": re-scan the catalog + // and confirm the thing this request actually needed -- a runtime at this version, paired + // with this device type -- is now present before reporting success. Either failure mode + // reports `component-install-failed`, never `component-installed`. + const refreshed = await this.#loadCatalog(); + const runtime = findInstalledRuntime(refreshed, osVersion); if (runtime === undefined) { - throw new IosRuntimeMissingError(request.osVersion ?? "default"); + const message = `xcodebuild reported success but iOS ${osVersion} is still not installed`; + this.#reportVerificationFailure(osVersion, startedAt, message, options.requesterId); + throw new DriverCrashError(message); + } + // A version match alone is not enough: the same pairing check that gates an + // already-installed runtime above must also gate a freshly downloaded one -- a version can + // be on disk and still not pair with this specific device type. + if (!runtime.supportedDeviceTypeIds.has(deviceType.identifier)) { + this.#reportVerificationFailure( + osVersion, + startedAt, + `iOS ${osVersion} installed but does not pair with ${deviceType.name}`, + options.requesterId, + ); + throw new IosRuntimeUnpairedError(deviceType.name, osVersion); + } + this.#onDiagnostic?.({ + componentId: osVersion, + durationMs: this.#clock.now() - startedAt, + kind: "component-installed", + ...(options.requesterId === undefined ? {} : { requesterId: options.requesterId }), + }); + return this.#commitResolution(deviceType, runtime); + } + + /** + * No version requested: defaults to the newest *installed* runtime that both falls in the + * model's range and actually pairs with it (`supportedDeviceTypes`) -- not the newest + * installed runtime overall, which may have dropped this model (iOS 26 dropping iPhone + * XS/XR support is the motivating case). + */ + // fallow-ignore-next-line complexity -- the download-target decision and post-download verification are one resolution attempt. + async #resolveDefaultRuntime( + deviceType: DeviceType, + catalog: SimctlCatalog, + options: { readonly allowDownload: boolean; readonly requesterId?: string }, + ): Promise { + const paired = pairedInstalledRuntime(catalog, deviceType); + if (paired !== undefined) { + return this.#commitResolution(deviceType, paired); } + if (!options.allowDownload) { + throw new IosRuntimeMissingError( + "default", + `No installed iOS runtime pairs with ${deviceType.name}; pass --allow-download (or ` + + `set downloads.policy) to download a compatible runtime`, + ); + } + + let componentId: string; + let startedAt: number; + if (isUnboundedMax(deviceType.maxRuntimeVersion)) { + // No upper bound on this model's pairing range: any released version works, so there is + // nothing more specific to ask for than "latest". + componentId = "latest"; + startedAt = await this.#downloadRuntime( + componentId, + ["-downloadPlatform", "iOS"], + options.requesterId, + ); + } else { + const major = majorVersionString(deviceType.maxRuntimeVersion); + componentId = major; + try { + startedAt = await this.#downloadRuntime( + major, + ["-downloadPlatform", "iOS", "-buildVersion", major], + options.requesterId, + ); + } catch (error: unknown) { + // A disk preflight failure or a typed "nothing to do here" (e.g. a concurrent caller's + // RuntimeMissingError) is meaningful on its own and must reach the caller unchanged -- + // wrapping it in a DriverCrashError below would bury a clean, actionable error under an + // opaque "could not download" one. + if (error instanceof InsufficientDiskSpaceError || error instanceof RuntimeMissingError) { + throw error; + } + throw new DriverCrashError( + `Could not download a default iOS runtime for ${deviceType.name} (tried ${major}): ` + + `${errorMessage(error)}; pass --os to request an exact release`, + ); + } + } + + // Same verified-fact requirement as the exact-version path: only report `component-installed` + // once a paired runtime for this device type is actually present in a re-scanned catalog. + const refreshed = await this.#loadCatalog(); + const runtime = pairedInstalledRuntime(refreshed, deviceType); + if (runtime === undefined) { + const message = + `xcodebuild reported success but no installed iOS runtime pairs with ` + + `${deviceType.name} yet`; + this.#reportVerificationFailure(componentId, startedAt, message, options.requesterId); + throw new DriverCrashError(message); + } + this.#onDiagnostic?.({ + componentId, + durationMs: this.#clock.now() - startedAt, + kind: "component-installed", + ...(options.requesterId === undefined ? {} : { requesterId: options.requesterId }), + }); + return this.#commitResolution(deviceType, runtime); + } + + /** + * Reports the terminal `component-install-failed` diagnostic for the "xcodebuild exited 0 but + * post-download verification didn't find what this request needed" case -- pairing failure or + * outright absence. `#installComponent` already reports `component-install-failed` for a + * nonzero xcodebuild exit; this covers the other way an install attempt can fail to produce a + * usable component. + */ + #reportVerificationFailure( + componentId: string, + startedAt: number, + message: string, + requesterId: string | undefined, + ): void { + this.#onDiagnostic?.({ + componentId, + durationMs: this.#clock.now() - startedAt, + error: message, + kind: "component-install-failed", + ...(requesterId === undefined ? {} : { requesterId }), + }); + } + + #commitResolution(deviceType: DeviceType, runtime: Runtime): DeviceSpec { const spec: DeviceSpec = { model: deviceType.name, osVersion: runtime.version, @@ -128,6 +362,95 @@ export class IosSimctlDriver implements Driver { return spec; } + /** + * Runs `xcodebuild -downloadPlatform iOS [-buildVersion ]`, deduping concurrent + * callers that ask for the exact same invocation behind one in-flight promise -- mirrors the + * Android driver's `#locks` pattern, sized to a single component instead of a whole device. + * The map entry is removed once the download settles (success or failure), so a later, + * non-concurrent call starts a fresh attempt rather than replaying a stale result. `componentId` + * is the runtime version being installed ("latest" for a bare `-downloadPlatform iOS`, the bare + * major version for the bounded-default case) -- reported on `component.install-*`, never + * parsed back out of `args`. Resolves to the `started` timestamp on success rather than + * `void`: the caller needs it to compute an accurate `durationMs` once its own post-download + * catalog re-scan confirms (or fails to confirm) the component it actually needed. + */ + async #downloadRuntime( + componentId: string, + args: readonly string[], + requesterId: string | undefined, + ): Promise { + const key = args.join(""); + const inFlight = this.#downloadLocks.get(key); + if (inFlight !== undefined) { + return inFlight; + } + + const promise = this.#installComponent(componentId, args, requesterId).finally(() => { + if (this.#downloadLocks.get(key) === promise) { + this.#downloadLocks.delete(key); + } + }); + this.#downloadLocks.set(key, promise); + return promise; + } + + /** + * Disk preflight (via the shared `DiskSpaceGuard`, released once `xcodebuild` settles either + * way), then `xcodebuild`, wrapped with `component.install-*` diagnostics. A preflight failure + * is reported before any diagnostic fires -- no install was actually attempted, so there is + * nothing to report as started or failed. + */ + async #installComponent( + componentId: string, + args: readonly string[], + requesterId: string | undefined, + ): Promise { + const release = await this.#diskSpaceGuard.reserve( + this.#filesystem, + this.platform, + IOS_RUNTIME_MIN_FREE_BYTES, + this.#coreSimulatorRoot, + ); + try { + this.#onDiagnostic?.({ + componentId, + kind: "component-install-started", + ...(requesterId === undefined ? {} : { requesterId }), + }); + const startedAt = this.#clock.now(); + try { + await this.#xcodebuildOrThrow(args); + } catch (error: unknown) { + this.#onDiagnostic?.({ + componentId, + durationMs: this.#clock.now() - startedAt, + error: stableError(error), + kind: "component-install-failed", + ...(requesterId === undefined ? {} : { requesterId }), + }); + throw error; + } + // No `component-installed` here: xcodebuild exiting 0 only means the tool claims success, + // not that the catalog now has what a specific caller needed (an exact version, or one + // that pairs with a specific device type). The caller re-scans and reports the terminal + // fact itself -- see `#reportVerificationFailure` and its call sites. + return startedAt; + } finally { + release(); + } + } + + async #xcodebuildOrThrow(args: readonly string[]): Promise { + const result = await this.#processRunner.run("xcodebuild", args, { + timeoutMs: this.#downloadTimeoutMs, + }); + if (result.code !== 0) { + throw new DriverCrashError( + `xcodebuild ${args.join(" ")} failed: ${result.stderr || result.stdout}`, + ); + } + } + async provision(spec: DeviceSpec): Promise { this.#requireIosPlatform(spec.platform); const resolved = await this.#resolvedSpec(spec); @@ -467,9 +790,64 @@ export class IosSimctlDriver implements Driver { } class IosRuntimeMissingError extends RuntimeMissingError { - constructor(osVersion: string) { + constructor(osVersion: string, message: string) { super("ios", osVersion); - this.message = `iOS runtime ${osVersion} is not installed; install it via Xcode`; + this.message = message; + } +} + +class IosUnknownModelError extends UnknownModelError { + constructor(model: string) { + super("ios", model); + this.message = `Unknown ios model: ${model}; a newer Xcode version may add this device`; + } +} + +/** + * A requested OS version outside the model's `[minRuntimeVersion, maxRuntimeVersion]` pairing + * range. Extends `RuntimeMissingError` (rather than living as an unrelated class) so it flows + * through the daemon/CLI exactly like `IosRuntimeMissingError` already does -- same error code, + * same exit status -- without either needing to learn a new type. Unlike a missing runtime, no + * download can ever fix this, which is why the message states the supported range instead of + * pointing at `--allow-download`. + */ +class IosVersionOutOfRangeError extends RuntimeMissingError { + constructor(model: string, requested: string, deviceType: DeviceType) { + super("ios", requested, { downloadable: false }); + const range = formatVersionRange(deviceType.minRuntimeVersion, deviceType.maxRuntimeVersion); + this.message = `${model} supports iOS ${range}; iOS ${requested} is out of range`; + } +} + +/** + * The requested runtime is installed and its version falls in the model's declared range, but + * the runtime's own `supportedDeviceTypes` (authoritative once it is actually on disk) does not + * include this model -- e.g. a device Apple dropped support for in a later point release of an + * OS it otherwise still ships. Extends `RuntimeMissingError` for the same reason + * `IosVersionOutOfRangeError` does (shared error code / exit status), and sets `downloadable: + * false` for the same reason: the runtime is already installed, so downloading it again changes + * nothing. + */ +class IosRuntimeUnpairedError extends RuntimeMissingError { + constructor(model: string, requested: string) { + super("ios", requested, { downloadable: false }); + this.message = `iOS ${requested} is installed but does not support ${model}`; + } +} + +/** + * A requested version predates Xcode's automatic download support (`xcodebuild + * -downloadPlatform` only reaches back to iOS 16.0 -- see `IOS_DOWNLOAD_FLOOR`). Not a driver + * crash: nothing went wrong, the request is simply outside what `--allow-download` can ever do. + * `RuntimeMissingError` with `downloadable: false` reports that distinction the same way the + * out-of-range and unpaired-runtime errors do, rather than surfacing as an opaque internal error. + */ +class IosDownloadFloorError extends RuntimeMissingError { + constructor(requested: string) { + super("ios", requested, { downloadable: false }); + this.message = + `iOS ${requested} predates Xcode's automatic download support (introduced for iOS ` + + `16.0 and newer); install it manually via Xcode`; } } @@ -527,8 +905,13 @@ function parseCatalog(value: unknown): SimctlCatalog { const deviceTypes = value.devicetypes.flatMap(parseDeviceType); const runtimes = value.runtimes.flatMap(parseRuntime); - if (deviceTypes.length === 0 || runtimes.length === 0) { - throw new DriverCrashError("Invalid simctl list JSON: no usable device types or runtimes"); + // Device types come from the Xcode install itself and are never empty on a working + // toolchain, so an empty list here means the JSON was malformed. Runtimes are different: a + // fresh Xcode with zero simulator runtimes installed is a normal, if unusual, starting state + // -- and it must be able to reach the download-latest path in `#resolveDefaultRuntime` rather + // than being rejected here before any resolution is attempted. + if (deviceTypes.length === 0) { + throw new DriverCrashError("Invalid simctl list JSON: no usable device types"); } return { deviceTypes, runtimes }; @@ -539,7 +922,14 @@ function parseDeviceType(value: unknown): readonly DeviceType[] { return []; } - return [{ identifier: value.identifier, name: value.name }]; + return [ + { + identifier: value.identifier, + maxRuntimeVersion: versionIntOr(value.maxRuntimeVersion, UNBOUNDED_VERSION), + minRuntimeVersion: versionIntOr(value.minRuntimeVersion, 0), + name: value.name, + }, + ]; } function parseRuntime(value: unknown): readonly Runtime[] { @@ -559,11 +949,89 @@ function parseRuntime(value: unknown): readonly Runtime[] { identifier: value.identifier, isAvailable: value.isAvailable, name: value.name, + supportedDeviceTypeIds: parseSupportedDeviceTypeIds(value.supportedDeviceTypes), version: value.version, }, ]; } +function versionIntOr(value: unknown, fallback: number): number { + return typeof value === "number" && Number.isFinite(value) ? value : fallback; +} + +function parseSupportedDeviceTypeIds(value: unknown): ReadonlySet { + if (!Array.isArray(value)) { + return new Set(); + } + const ids = value + .filter(isRecord) + .map((entry) => entry.identifier) + .filter((identifier): identifier is string => typeof identifier === "string"); + return new Set(ids); +} + +function findInstalledRuntime(catalog: SimctlCatalog, version: string): Runtime | undefined { + return catalog.runtimes.find((runtime) => runtime.isAvailable && runtime.version === version); +} + +/** Installed, in the model's range, and pairs with it -- the newest of those, or none. */ +function pairedInstalledRuntime( + catalog: SimctlCatalog, + deviceType: DeviceType, +): Runtime | undefined { + const candidates = catalog.runtimes.filter( + (runtime) => + runtime.isAvailable && + runtime.supportedDeviceTypeIds.has(deviceType.identifier) && + isVersionInRange(runtime.version, deviceType), + ); + return newestRuntime(candidates); +} + +/** `simctl`'s `0xAABBCC` encoding -> `[major, minor, patch]`. */ +function decodeVersionTriple(encoded: number): readonly [number, number, number] { + return [(encoded >> 16) & 0xff, (encoded >> 8) & 0xff, encoded & 0xff]; +} + +function formatDecodedVersion(encoded: number): string { + const [major, minor, patch] = decodeVersionTriple(encoded); + return patch === 0 ? `${major}.${minor}` : `${major}.${minor}.${patch}`; +} + +function isUnboundedMax(encoded: number): boolean { + return encoded >= UNBOUNDED_VERSION; +} + +function formatVersionRange(min: number, max: number): string { + const minLabel = formatDecodedVersion(min); + return isUnboundedMax(max) ? `${minLabel}+` : `${minLabel}-${formatDecodedVersion(max)}`; +} + +function versionTriple(version: string): readonly [number, number, number] { + const parts = version.split(".").map(versionPart); + return [parts[0] ?? 0, parts[1] ?? 0, parts[2] ?? 0]; +} + +function versionOrdinal(triple: readonly [number, number, number]): number { + return triple[0] * 1_000_000 + triple[1] * 1_000 + triple[2]; +} + +function isVersionInRange(version: string, deviceType: DeviceType): boolean { + const ordinal = versionOrdinal(versionTriple(version)); + return ( + ordinal >= versionOrdinal(decodeVersionTriple(deviceType.minRuntimeVersion)) && + ordinal <= versionOrdinal(decodeVersionTriple(deviceType.maxRuntimeVersion)) + ); +} + +function isTooOldToDownload(version: string): boolean { + return versionOrdinal(versionTriple(version)) < versionOrdinal(IOS_DOWNLOAD_FLOOR); +} + +function majorVersionString(encoded: number): string { + return String(decodeVersionTriple(encoded)[0]); +} + function newestRuntime(runtimes: readonly Runtime[]): Runtime | undefined { return [...runtimes].sort((left, right) => compareVersions(left.version, right.version)).at(-1); } diff --git a/src/ports/filesystem.ts b/src/ports/filesystem.ts index 7ac0dfb..e925a60 100644 --- a/src/ports/filesystem.ts +++ b/src/ports/filesystem.ts @@ -175,7 +175,10 @@ export class MemoryFilesystem implements Filesystem { #entryAt(path: string): MemoryEntry { const entry = this.#entries.get(path); if (entry === undefined) { - throw new Error(`No such file or directory: ${path}`); + // Carries `code: "ENOENT"` like Node's real fs errors, so callers that branch on + // `isMissingPathError` (rather than swallowing every read failure) behave the same + // against this fake as they do against `NodeFilesystem`. + throw enoent(path); } return entry; @@ -194,6 +197,14 @@ function parentPath(path: string): string { return lastSeparator <= 0 ? "/" : path.slice(0, lastSeparator); } -function isMissingPathError(error: unknown): error is NodeJS.ErrnoException { +export function isMissingPathError(error: unknown): error is NodeJS.ErrnoException { return typeof error === "object" && error !== null && "code" in error && error.code === "ENOENT"; } + +function enoent(path: string): NodeJS.ErrnoException { + const error = new Error( + `ENOENT: no such file or directory, open '${path}'`, + ) as NodeJS.ErrnoException; + error.code = "ENOENT"; + return error; +} diff --git a/src/ports/index.ts b/src/ports/index.ts index ff9cec2..0eebdc7 100644 --- a/src/ports/index.ts +++ b/src/ports/index.ts @@ -1,4 +1,9 @@ -export { type Filesystem, MemoryFilesystem, NodeFilesystem } from "./filesystem.js"; +export { + type Filesystem, + isMissingPathError, + MemoryFilesystem, + NodeFilesystem, +} from "./filesystem.js"; export { resolveSimlockHome } from "./paths.js"; export { type DaemonLauncher, FakeDaemonLauncher, NodeDaemonLauncher } from "./daemon-launcher.js"; export { @@ -29,13 +34,13 @@ export type { LogRecord } from "./logger.js"; export { CryptoIdGenerator, type IdGenerator } from "./id-generator.js"; export { FakeSystemStats, NodeSystemStats, type SystemStats } from "./system-stats.js"; export { FakeParentWatch, NodeParentWatch, type ParentWatch } from "./parent-watch.js"; -// fallow-ignore-next-line unused-type -- public handle contract returned by ParentWatch.watch(). export type { ParentWatchHandle } from "./parent-watch.js"; export { NodeProcessRunner, type ProcessHandle, type ProcessResult, type ProcessRunner, + type ProcessRunOptions, ScriptedProcessRunner, type ScriptedProcessExpectation, } from "./process-runner.js"; diff --git a/src/ports/process-runner.test.ts b/src/ports/process-runner.test.ts index f6ef90a..ac68d17 100644 --- a/src/ports/process-runner.test.ts +++ b/src/ports/process-runner.test.ts @@ -97,6 +97,25 @@ describe("NodeProcessRunner", () => { ).resolves.toEqual({ code: null, stderr: "", stdout: "" }); }); + // Real end-to-end coverage of the SIGTERM -> SIGKILL escalation: a child that installs a + // no-op SIGTERM handler survives the initial signal `run()` sends on timeout, so this only + // resolves at all if the follow-up SIGKILL actually lands once the grace period elapses. + // `SIGTERM_TO_SIGKILL_GRACE_MS` is a fixed 10s (see process-runner.ts), hence the generous + // per-test timeout below -- there is no faster way to prove the real runner's own timers + // actually escalate without mocking out `child_process`, which every other test in this + // `describe` block deliberately avoids. + it("escalates to SIGKILL when a timed-out process ignores SIGTERM", async () => { + const runner = new NodeProcessRunner(); + + await expect( + runner.run( + process.execPath, + ["-e", "process.on('SIGTERM', () => {}); setInterval(() => {}, 1_000)"], + { timeoutMs: 100 }, + ), + ).resolves.toEqual({ code: null, stderr: "", stdout: "" }); + }, 15_000); + it("settles wait() when a detached grandchild keeps the stdio pipe open after the process exits", async () => { const runner = new NodeProcessRunner(); // The child forks its own detached grandchild that inherits our pipe's write diff --git a/src/ports/process-runner.ts b/src/ports/process-runner.ts index 2c4f4bd..1dee32f 100644 --- a/src/ports/process-runner.ts +++ b/src/ports/process-runner.ts @@ -20,10 +20,23 @@ const EXIT_TO_CLOSE_GRACE_MS = 1_000; // may be incomplete; nothing else is waiting on it by then. const EXIT_TO_CLOSE_MAX_DEFERRAL_MS = 5_000; +// A hard bound on how long `run()` waits after a timeout-triggered SIGTERM before escalating to +// SIGKILL. A child that ignores SIGTERM (or is itself stuck in an uninterruptible wait) would +// otherwise hold the caller's `await process.wait()` open forever -- exactly the unbounded wait +// this constant exists to cap. Fixed rather than derived from `timeoutMs`: it is a +// termination-cleanup budget, not a scaled fraction of the operation's own timeout. +const SIGTERM_TO_SIGKILL_GRACE_MS = 10_000; + export interface ProcessRunOptions { readonly timeoutMs?: number; readonly env?: NodeJS.ProcessEnv; readonly cwd?: string; + /** + * Written to the child's stdin and then closed. For a CLI that reads an interactive prompt + * from stdin (e.g. `sdkmanager --licenses`'s per-license `y/N`) rather than accepting a flag. + * Omitted means stdin is left open and unwritten, exactly as before this option existed. + */ + readonly input?: string; } export interface ProcessResult { @@ -56,6 +69,7 @@ export class NodeProcessRunner implements ProcessRunner { options: ProcessRunOptions = {}, ): Promise { const process = this.spawn(command, args, options); + let killTimeout: NodeJS.Timeout | undefined; const timeout = options.timeoutMs === undefined ? undefined @@ -65,6 +79,18 @@ export class NodeProcessRunner implements ProcessRunner { } catch { // The child may have exited between the timer firing and the kill. } + // SIGTERM is a request, not a guarantee -- a child that ignores it (or is itself + // hung) must not be able to keep this `run()` call waiting indefinitely. Escalate to + // SIGKILL if it hasn't exited within the grace period; cleared below like `timeout` + // itself the moment `process.wait()` actually settles, so a child that does exit + // promptly after SIGTERM never sees the follow-up signal. + killTimeout = setTimeout(() => { + try { + process.kill("SIGKILL"); + } catch { + // The child may have exited between the timer firing and the kill. + } + }, SIGTERM_TO_SIGKILL_GRACE_MS); }, options.timeoutMs); try { @@ -73,6 +99,9 @@ export class NodeProcessRunner implements ProcessRunner { if (timeout !== undefined) { clearTimeout(timeout); } + if (killTimeout !== undefined) { + clearTimeout(killTimeout); + } } } @@ -84,6 +113,10 @@ export class NodeProcessRunner implements ProcessRunner { stdio: "pipe", }); + if (options.input !== undefined) { + child.stdin?.end(options.input); + } + return new NodeProcessHandle(child); } }