fix(devices): resolve the Android SDK per spawn, not once at startup - #11647
thelonewander3r wants to merge 1 commit into
Conversation
`LocalDeviceHost.make` resolved `androidSdk` once and captured the result in
`run` and in the environment handed to the hub and the agent-device daemon.
`platformReason` re-resolves on every availability check, so the two drift
apart as soon as the SDK is installed, moved, or repaired after the server
started: availability reports Android as usable, while `run` still spawns a
bare `emulator` that is not on PATH.
`fetchDevices` calls `run("emulator", ["-list-avds"])` whenever Android reports
available and turns a non-zero exit into a hard error, so the whole device list
fails — iOS simulators included — with "Device list failed: The device command
failed (exit code 127)", and stays that way until the app restarts. The hub and
daemon are also left without ANDROID_HOME and without the SDK on their PATH.
Move the command and environment resolution into `hostSpawn`, evaluated per
spawn, so spawns and availability always read the same SDK.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a narrowly scoped device-host bug fix that resolves the Android SDK per command, preventing stale startup state from breaking emulator discovery after SDK changes. Existing startup behavior remains intact, and targeted tests cover absolute-path execution and dynamic SDK discovery. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughLocalDeviceHost now resolves the Android SDK, executable command, and environment for each spawn. Emulator commands use the current SDK when available. Host, hub, daemon, and run paths consume the per-spawn results. Tests cover command mapping, environment setup, and SDK rediscovery. ChangesLocalDeviceHost spawn resolution
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant LocalDeviceHost
participant androidSdk
participant FileSystem
participant ChildProcess
LocalDeviceHost->>androidSdk: Resolve SDK for spawn
androidSdk->>FileSystem: Check current SDK availability
LocalDeviceHost->>LocalDeviceHost: Build command and environment
LocalDeviceHost->>ChildProcess: Spawn resolved command with environment
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The per-launch SDK resolution is propagated to the affected process paths, including SDK rediscovery, with no concrete merge-blocking issue identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
What Changed
LocalDeviceHost.makeresolvedandroidSdkonce at construction and captured the result inrunand in the environment handed to the device hub and the agent-device daemon. That resolution now happens per spawn, in a new module-levelhostSpawn.No behaviour changes when the SDK is present before the server starts.
Why
platformReasonre-resolves the SDK on every availability check, whilerunheld a copy from startup. The two drift apart as soon as the SDK is installed, moved, or repaired after the server started, and then disagree permanently:With
sdk.rootstillnull,runspawns a bareemulatorthat is not on the server's PATH.ProcessRunnermaps the spawn failure to exit 127, andfetchDevicesturns any non-zero exit into a hard error — so the whole device list fails, iOS simulators included, and stays that way until the app restarts.Observed on a machine with a complete, working SDK at
~/Library/Android/sdk:Android reports available and the same call reports failed. The underlying cause was
ProcessSpawnError: Failed to spawn process 'emulator'— note the bare name, whichrunonly passes whensdk.rootis falsy.The same stale value reaches the hub and the daemon, which were spawned with no
ANDROID_HOMEand without the SDK on their PATH, so they cannot see emulators either.Resolving per spawn is what keeps spawns and availability reading the same SDK. The cost is one extra SDK probe per spawn, which is the same work
platformAvailabilityalready does two to three times per refresh.Tests
Three cases in
LocalDeviceHost.test.tscovering the absolute emulator path, passthrough for other commands, and an SDK that appears after an earlier spawn resolved nothing. Reintroducing the cache makes the third fail.🤖 Generated with Claude Code
Summary by CodeRabbit