feat(android): Simlock's own AVD home and adb server (3/5) - #82
Merged
V3RON merged 2 commits intoSep 4, 2026
Merged
Conversation
Android has no `simctl --set`, so containment is built rather than inherited: AVDs live in a root this driver created and proved, every `adb`, `emulator` and `avdmanager` call carries `ANDROID_AVD_HOME` and `ANDROID_ADB_SERVER_PORT` through one insertion point, and console ports move to 5586-5682, above the 5585 ceiling a default adb server scans. `listManaged` answers from membership in that root -- a name prefix is not evidence, and being visible to our server is not evidence either. That last point is why this commit also corrects decision 4 of the ADR. The record specifies `ADB_LOCAL_TRANSPORT_MAX_PORT=5683` alone, but adb's scan has no minimum-port variable: the ceiling only widens a sweep that always starts at 5555, so the configuration as written would have Simlock's server discover and connect to the user's own emulators -- two servers contending for one device, the accident this ADR exists to prevent, running the other way. `ADB_EMU=0` is what actually contains it. Our emulators still attach because the emulator registers itself with `host:emulator:<port>`, and Simlock sends that registration too, which replaces the reconnect the scanner would otherwise have retried. The ceiling stays as belt-and-braces for a build that ignores `ADB_EMU`. Sources are cited in the ADR beside the correction. The server is supervised by pid because it has to be: it is started with `ADB_REJECT_KILL_SERVER=1`, so an agent's reflexive `adb kill-server` cannot detach every leased emulator at once -- and neither can Simlock. `adb-server.json` records the pid, deliberately outside `state.json`, since a corrupt registry is exactly when a leftover server must still be reapable. It is run as `nodaemon server` so the recorded pid is the server rather than a launcher that exits. A pid is not an identity, though: before any signal the process is confirmed to actually be adb, so a recycled pid in a stale record is dropped rather than killed. An occupied port fails the driver closed instead of attaching to whatever is listening, which could be Android Studio's server. Refs: #73, #70 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BuTqSL7fKJVRbFytNvZP7X
An adversarial review found this design defended the crash it anticipated and broke on the orderly shutdown it did not. Devices are meant to survive `daemon stop`: releasing a lease hands the device to the warm pool, and convergence only stops the excess. But shutdown reaps the adb server, and an emulator announces itself exactly once, to the server that was running when it booted. With the scanner off, nothing found them again -- `adb devices` came back empty, every running AVD reported `stopped`, no orphan finding could ever name them, and the port allocator handed 5586 to a new device that then could not bind. That is the occupancy defect the ADR set out to fix, arriving from the other side. Simlock now does deliberately, for its own console range, what adb's scanner would have done for everyone's: after starting or adopting a server it announces every port in 5586-5682 and lets adb re-connect what is actually there. The eager announcement after `spawn` is gone rather than moved -- adb answers by connecting back to a port the emulator has not opened yet, so it could only ever fail. The identity check that guards every signal read `ps -o comm=`, which strips the arguments that make a server ours. It proved "some adb", and "some adb" on a developer's machine is most likely the user's own server holding a recycled pid: a stale record plus a quiet port would have SIGKILLed the adb server Android Studio was using. It now requires the full command line -- an adb binary, our port, and `nodaemon`. Failing after the spawn was worse than failing before it. The child was never `unref`ed, so the daemon hung on an exit it had already decided to make; the record was written after the wait, so the pid was lost exactly when a crash made it matter; and a disk error while writing it took down iOS as well. The record is now written as soon as the pid exists, every failure kills the child and refuses only Android, and the one state with no automatic recovery says how to recover from it. A spawn that never produced a process crashed the daemon through an unhandled `error` event, which no caller could catch. An inconclusive `ps` at shutdown deleted the record and left the server running, which turned the first clean shutdown on a host without `ps` into a permanently dead platform; an answer nobody could read is now no answer. Also corrects the reason given for `ADB_LOCAL_TRANSPORT_MAX_PORT` in the ADR: the sweep starts at a hard-coded 5555, so the ceiling never bounded it away from the user's emulators. It is kept for what it actually does. Refs: #73, #70 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BuTqSL7fKJVRbFytNvZP7X
V3RON
added a commit
that referenced
this pull request
Sep 4, 2026
…wrappers (4/5) (#83) Fourth of five stacked PRs implementing [ADR 0001](https://github.com/callstackincubator/simlock/blob/feat/owned-device-roots/docs/adr/0001-simlock-owned-device-roots.md). Based on #82. 1. [#80](#80) owned device root validation and its ports 2. [#81](#81) iOS: `simctl --set`, membership-based `listManaged` 3. [#82](#82) Android: `ANDROID_AVD_HOME`, private supervised adb server 4. **lease environment and the `simctl` / `adb` passthroughs** ← this PR 5. doctor: `--purge-orphans`, legacy pre-root devices, docs status ## What this is Containment cuts both ways, and PRs 2 and 3 only built the outward half. A simulator in Simlock's device set and an emulator on Simlock's adb server are unreachable with a bare `simctl` or `adb` — which is the point for everyone else, and a problem for the agent holding the lease. Until this lands, `simlock lease --json` returns a `udid` no documented workflow can address (ADR decision 7). - **A grant carries an `environment`** the driver built and the core never reads — the device-set path on iOS, the adb server port on Android. Set at the single `LeaseGrant` construction site, forwarded verbatim, so a third driver contributes its own scoping without a core edit (architecture rule 2). - **`--export-env`** prints it as shell exports for `eval`, quoted so a path with a space or an apostrophe survives the round trip. The e2e test asserts that through a real `/bin/sh`, not a string comparison. - **`simlock simctl` and `simlock adb`** inject the scoping and pass every other argument through. Each driver owns the verbs its wrapper refuses, because handing those back would restore the exact capability this work removes. - The client parses `environment` leniently, and only that field: an older daemon sends none, and a grant is still worth having without one. ## Review round (third commit) An adversarial review found the most serious defect in the stack so far: **`simlock simctl --profiles /tmp erase all` was proxied.** The subcommand was taken as the first argument not starting with `-`, and simctl's two global options put their value in a *separate* argv entry — so `/tmp` read as the subcommand, no refusal fired, and the daemon returned `xcrun simctl --set <simlockRoot> --profiles /tmp erase all`. **Simlock supplied the containment path itself**, erasing every simulator in the root including devices under another agent's live lease. The barrier was supposed to be knowing that path. A caller-supplied `--set`/`--profiles` is now refused outright, which makes "the first non-flag argument is the subcommand" true by construction. Worth stating honestly: `simlock simctl list devices -j` prints `dataPath` for every device, so a *determined* user learns the root anyway, and the ADR says this is not a security boundary. What the refusal buys is the typo-and-reflex guard — and that was broken. **The refusal list was a list of words, not a model of the rule.** It let through verbs worse than the ones it stopped: `simctl shutdown all` (the iOS analogue of `adb kill-server`, which Android refuses for exactly that reason — it stops every device Simlock believes is running, for every agent, burning each lease's recovery budget), `adb emu avd stop`, and `adb emu avd snapshot delete` (which destroys the baseline every later reclaim restores from, silently degrading reclaims from seconds to a full wipe). Bare tooling cannot reach any of those devices, so each was a returned capability. `simctl runtime delete` is not in that class but is now refused too — a wrapper offered as the safe path should not proxy something it cannot undo and Simlock will not re-download. Also fixed: `--export-env` escaped values against every pathological input and did not escape keys at all, so a driver-supplied key could carry a command into the `eval` the flag exists for (confirmed by execution); and against an older daemon it exited 0 having printed nothing, leaving a lease committed, TTL-bound and unnameable. ## Not amended ADR decision 7's `(delete, erase, create, emu kill)` is illustrative; the added refusals apply that decision rather than change it, so the record stands as written. ## Worth your judgement **The passthrough requires no lease.** Any local process can `simlock adb shell …` against a device another agent holds. That may be intended operator convenience, but it makes the refusal list the only thing between one agent and another agent's device. ## Verification `pnpm run check` green: typecheck, typecheck:e2e, lint, format, 927 unit tests, e2e 38 passed / 1 expected fail / 3 skipped (five new cases, including the real-`/bin/sh` export round trip). `fallow audit` clean across 34 changed files. The `slow-*` lanes gained passthrough and environment assertions but **cannot run here** — no macOS, no Xcode, no Android SDK. One unrelated defect surfaced and was deliberately not absorbed: `daemon stop` does not exit while a detached lease is outstanding. It reproduces on `feat/owned-device-roots` with none of this stack applied; the new e2e test releases its lease with a comment pointing at it. Refs #73, #70 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01BuTqSL7fKJVRbFytNvZP7X --- _Generated by [Claude Code](https://claude.ai/code/session_01BuTqSL7fKJVRbFytNvZP7X)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
V3RON
added a commit
that referenced
this pull request
Sep 4, 2026
Fifth of five stacked PRs implementing [ADR 0001](https://github.com/callstackincubator/simlock/blob/feat/owned-device-roots/docs/adr/0001-simlock-owned-device-roots.md). Based on #83. **This is the PR that marks the ADR implemented.** 1. [#80](#80) owned device root validation and its ports 2. [#81](#81) iOS: `simctl --set`, membership-based `listManaged` 3. [#82](#82) Android: `ANDROID_AVD_HOME`, private supervised adb server 4. [#83](#83) lease environment and the scoped wrappers 5. **doctor: `--purge-orphans`, legacy pre-root devices, docs status** ← this PR ## What this is Every earlier PR made Simlock able to *prove* what it owns. This one makes it *destroy* on the strength of that proof, which is why the wiring matters more than the feature. **`doctor --purge-orphans`** reclaims a device that sits inside a validly-marked root with no registry record — almost always a daemon that died between creating a device and writing it down, and until now permanently unreclaimable, because registry-only destruction cannot reach what the registry has never heard of. It is the single opt-in exception safety rule 1 allows, and it is deliberately hard to reach by accident: its own flag rather than part of `--fix` (so a `doctor --fix` already running unattended in CI does not acquire a destructive behaviour on upgrade), a confirmation unless `--yes`, and no reachability from the reaper, a cleanup rule, or an idle tier. The reason it must stay opt-in is structural. Every central safety filter is written over registry records, and an orphan has none — so an orphan proposal bypasses the safety net rather than passing through it, including a device this very daemon is mid-provision on. That reasoning is written at the purge site. **Roots are re-proven before anything is destroyed.** Ownership is established once at startup and then trusted for the life of the process — fine for reporting, not fine for destroying, since a daemon up for days is one `mv` or symlink away from a root that now holds the user's own simulators. Every root a purge would touch is re-validated first via a new `Driver.revalidateRoot()`, and a refusal aborts the *whole* run rather than one platform, so a half-purge is impossible rather than merely unlikely. **Legacy pre-root devices** are reported as `legacy-device` rather than as vanished, and `--fix` destroys them through their old unscoped path. That is the one destruction that reaches outside an owned root, and it is permitted because the device is in the registry: registry-only destruction is satisfied by the record, not by the root (ADR Migration paragraph). ## Docs stop describing a future ADR 0001 status → `Accepted`; `device.orphan-purged` → `implemented`; the `(planned)` markers and "after ADR 0001 lands" notes out of `known-pitfalls.md`. Two doc-truth findings surfaced while flipping the status: - **`docs/ARCHITECTURE.md` was made untrue by this PR.** It said "conversely Simlock cannot address anything outside [the root]", which `destroyLegacy` breaks. Amended with the migration exception and the startup-proof / re-proof split rather than left standing as a false statement. - **A new pitfall entry**: a root's ownership is proven at startup and trusted for the daemon's life. The existing "accident boundary, not a security boundary" entry covers someone deliberately passing `--set`; it said nothing about the root being replaced under a running daemon. Nothing in the ADR's decisions needed changing, which is why the status flip is honest. ## Deliberate gaps - `DoctorReport`'s shape is unchanged, so a confirmed purge that was refused by root re-validation signals that only through the orphan findings still being present, plus a `daemon.log` line. An explicit "purge refused" in the report would be a reasonable follow-up. - `destroyLegacy` does not stop a running legacy emulator — doing so would mean driving the user's own adb server. - `docs/EVENTS.md` still carries a `> Status: **planned catalog**` header even though every row now reads `implemented`. Left alone as out of scope; worth a separate tidy. ## Verification `pnpm run check` green: typecheck, typecheck:e2e, lint, format, 949 unit tests, e2e 40 passed / 1 expected fail / 3 skipped (two new fake-driver flows: a purge with its confirmation gate, and a refused root destroying nothing). `fallow audit` clean across 24 changed files. Nothing in this PR has been exercised against a real `simctl` or `avdmanager` — no macOS, no Xcode, no Android SDK here. The `slow-*` lanes were reviewed and needed no change; they remain unrun. Refs #73, #70 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01BuTqSL7fKJVRbFytNvZP7X --- _Generated by [Claude Code](https://claude.ai/code/session_01BuTqSL7fKJVRbFytNvZP7X)_ Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third of five stacked PRs implementing ADR 0001. Based on #81.
simctl --set, membership-basedlistManagedANDROID_AVD_HOME, private supervised adb server ← this PRsimctl/adbpassthroughs--purge-orphans, legacy pre-root devices, docs statusWhat this is
Android has no
simctl --set, so containment is built rather than inherited. AVDs live in a root this driver created and proved; everyadb,emulatorandavdmanagercall carriesANDROID_AVD_HOMEandANDROID_ADB_SERVER_PORTthrough one insertion point; console ports move to 5586–5682, above the 5585 ceiling a default adb server scans.listManagedanswers from membership in that root — a name prefix is not evidence, and neither is being visible to our server.The server is supervised by pid because it has to be:
ADB_REJECT_KILL_SERVER=1means an agent's reflexiveadb kill-servercannot detach every leased emulator at once, and neither can Simlock. It runs asnodaemon serverso the recorded pid is the server rather than a launcher that exits, andadb-server.jsonlives outsidestate.jsonbecause a corrupt registry is exactly when a leftover server must still be reapable. An occupied port refuses the platform rather than attaching to whatever is listening, which could be Android Studio's.This PR amends ADR decision 4
The record specifies
ADB_LOCAL_TRANSPORT_MAX_PORT=5683alone. Verified against adb's source, that is incomplete and actively harmful: the emulator scan has no minimum-port variable, so the ceiling only widens a sweep that always starts at 5555.Simlock's server would therefore have discovered and connected to the user's own emulators, leaving two adb servers contending for one device — the accident this ADR exists to prevent, running the other way.
ADB_EMU=0is what actually contains it, and our emulators still attach because the emulator announces itself withhost:emulator:<port>. The ADR carries a dated Correction with the citations;CONFIGURATION.mdandknown-pitfalls.mdmatch.Containment is now symmetric: our consoles sit above the user's default ceiling so their server cannot see ours, and the scanner is off so ours never looks at theirs.
Review round (second commit)
An adversarial review found eight defects. The worst was a design flaw, not a slip:
A clean
daemon stoporphaned every running emulator, permanently. Devices are meant to survive a restart — releasing a lease hands the device to the warm pool. But shutdown reaps the adb server, and an emulator announces itself exactly once, to the server running when it booted. With the scanner off nothing found them again:adb devicesempty, every running AVD reportingstopped, no orphan finding able to name them (several GB of RSS each, forever), and the allocator re-issuing 5586 to a device that then could not bind — the occupancy defect this ADR set out to fix, arriving from the other side. The design defended the crash it anticipated and broke on the orderly shutdown it did not.Simlock now does deliberately, for its own bounded range, what adb's scanner would have done for everyone's: after starting or adopting a server it announces every console port in 5586–5682. That also makes the design independent of whether a running emulator re-announces itself, which cannot be tested here.
The pid identity check would have killed the user's adb server.
ps -o comm=strips the arguments that make a server ours, so it proved "some adb" — and on a developer's machine a recycled pid most likely belongs to the user's own server. It now requires an adb binary, our port, andnodaemon.Also fixed: a failure after spawn hung the daemon (the child was never
unrefed) and disabled Android permanently, because the record was written after the wait; a disk error writing that record took iOS down too; a spawn that never produced a process crashed the daemon through an unhandlederrorevent no caller could catch; the port probe believed any listener, so a cold-start race let the loser clobber the winner's record; and an inconclusivepsat shutdown deleted the record while leaving the server alive, turning the first clean shutdown on a host withoutpsinto a permanently dead platform.Not verified here
No macOS, no Android SDK, no
adb— every branch is unit-tested against fakes. Specifically unverified on real tooling: thathost:emulator:<port>re-attaches an emulator started under a previous server (the restart-recovery path — the first experiment worth running on a Mac); thatps -o args=renders the command line as assumed under BSDps; thatunref()lets the daemon exit with the server running; and both slow lanes.Verification
pnpm run checkgreen: typecheck, typecheck:e2e, lint, format, 865 unit tests, e2e (33 passed, 1 expected fail, 3 skipped — matching baseline).fallow auditclean across 28 changed files.Refs #73, #70
🤖 Generated with Claude Code
https://claude.ai/code/session_01BuTqSL7fKJVRbFytNvZP7X
Generated by Claude Code