Skip to content

fix(server): Device panel finds Android SDKs installed after startup - #21502

Open
flamboh wants to merge 2 commits into
pingdotgg:mainfrom
flamboh:t3/device-panel-android-linux
Open

flamboh wants to merge 2 commits into
pingdotgg:mainfrom
flamboh:t3/device-panel-android-linux

Conversation

@flamboh

@flamboh flamboh commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Note

🤖 Claude Opus 5.5 on behalf of Oliver

Who this affects

Anyone who installs the Android SDK while T3 Code is already running. That's the normal first-time flow: open the Device panel, see "Android SDK was not found", install Android Studio, come back.

Case Before After
SDK installed after the server started The panel says Android is ready, then listing fails with The device command failed (exit code 127). Nothing lists or opens until the server restarts. Emulators list and open.
No Java on the server's PATH (common for desktop and systemd launches) The panel says Android is ready, then shows [android-utils] Failed to run avdmanager list avd. The hub returns no devices, including ones already running. Uses Android Studio's bundled JDK if present. Otherwise Android shows as unavailable: "Android SDK Command-line Tools need Java 17 or later … set JAVA_HOME for the T3 Code server."

Found on a Linux machine where the SDK went into ~/Android/Sdk about two hours after the server started.

Why it broke

LocalDeviceHost looked for the SDK once, at server start, and built the hub's environment from that result. The availability check looks again on every call, so the panel showed "ready" while the hub and emulator -list-avds still ran without an SDK:

  • emulator ran as a bare command and failed to start. run reports any spawn failure as exit 127.
  • The hub got no ANDROID_HOME, so expo-device-hub fell back to its macOS default, ~/Library/Android/sdk, even on Linux.

Even with the SDK found, the hub lists AVDs with avdmanager, which is a Java program. When that fails, the hub returns an empty list, so listing by emulator -list-avds alone isn't enough.

Change

  • Resolve on use. The SDK and Java are looked up each time the hub or agent tools start, and on every device-tool command (run). This matches how command lookup already works in packages/shared/src/shell.ts, which deliberately doesn't cache misses so newly installed binaries are found.
  • Find a JDK. Order: JAVA_HOME, then java on PATH, then Android Studio's bundled JBR. These are the same candidates sshDeviceScript.ts already uses, plus /usr/local/android-studio and the snap install, added to both hosts. The result goes to the hub as JAVA_HOME.
  • Say when Java is missing. No JDK is now an Android unavailability reason, like the existing reasons for missing Platform-Tools, Emulator or Command-line Tools.
  • Docs. docs/user/devices.md lists the Java requirement.

SSH device hosts already re-ran their setup on every command, so they never had the startup bug. Only their JBR list changed. No contract or client change: the reason travels in the existing DevicePlatformAvailability.reason.

Not covered: a hub that is already running keeps the environment it started with. In practice that's a Mac whose hub started for iOS before the Android SDK was installed. The docs already say to restart after installing dependencies.

Related: #21425 changes which SDK root is chosen and notes the startup snapshot as a known limitation. This PR removes that snapshot. The two edit neighbouring code, so whichever lands second needs a small rebase.

Validation

  • vp test run apps/server/src/device: 103 tests pass. New cases cover an SDK and JDK installed after the host is created (the regression), the Java lookup order on Linux and Windows, and the no-Java message.
  • Server typecheck and targeted lint are clean.
  • I ran the real LocalDeviceHost against a real SDK and an AVD on Linux, using copies of the pinned device tools and a temp state directory:
    • With no Java: {"available":false,"reason":"Android SDK Command-line Tools need Java 17 or later, which was not found. …"}
    • With JAVA_HOME set: the hub listed T3_Pixel with "errors":[]. Booting it through the hub returned emulator-5554, booted: true.

macOS and Windows were not run end to end.

Made by Claude Opus 5.5 in Claude Code, running in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 19, 2026
Comment thread apps/server/src/device/LocalDeviceHost.ts Outdated
Comment thread apps/server/src/device/LocalDeviceHost.ts Outdated
@macroscopeapp

macroscopeapp Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at b43a572

Macroscope's review found this PR approvable — This is a focused device-host bug fix that refreshes Android SDK/JDK discovery when commands run, with regression tests for post-startup installation and cross-platform JDK lookup. The SSH adjustment and documentation are similarly scoped, with no API, schema, deployment, or security-sensitive changes.

You can add or adjust custom eligibility rules. Learn more.

@github-actions github-actions Bot added size:L 100-499 changed lines (additions + deletions). and removed size:M 30-99 changed lines (additions + deletions). labels Oct 19, 2026
@coderabbitai

coderabbitai Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e7eeb0d9-0ca3-4043-959a-35de5e2744f7
📥 Commits

Reviewing files that changed from the base of the PR and between 3443f02 and b43a572.

📒 Files selected for processing: 4
  • apps/server/src/device/LocalDeviceHost.test.ts
  • apps/server/src/device/LocalDeviceHost.ts
  • apps/server/src/device/sshDeviceScript.ts
  • docs/user/devices.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

Android device availability now checks for Java 17 or later and resolves Java from JAVA_HOME, PATH, or Android Studio’s bundled JDK. Device operations resolve the SDK and Java environment when used. The device setup documentation describes these Java requirements and sources.

Changes: Android Java support

Layer / File(s) Summary
Java discovery and availability
apps/server/src/device/LocalDeviceHost.ts, apps/server/src/device/sshDeviceScript.ts, apps/server/src/device/LocalDeviceHost.test.ts, docs/user/devices.md
Android availability checks for Java 17 or later. Java discovery uses JAVA_HOME, PATH, and Android Studio JDK locations. SSH fallback logic, tests, and setup instructions cover the discovery sources.
Resolved environment in device operations
apps/server/src/device/LocalDeviceHost.ts, apps/server/src/device/LocalDeviceHost.test.ts
SDK and Java resolution now occur when Android tools are used. Host operations and device commands receive the resolved environment. Tests cover exported JAVA_HOME and SDK discovery after host startup.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant LocalDeviceHost
  participant androidTools
  participant filesystem
  participant processRunner
  LocalDeviceHost->>androidTools: Resolve SDK and Java
  androidTools->>filesystem: Inspect SDK and JDK paths
  filesystem-->>androidTools: Return discovered paths
  androidTools-->>LocalDeviceHost: Return tools and host environment
  LocalDeviceHost->>processRunner: Run device command with resolved environment
Loading

Suggested reviewers: juliusmarminge, pixpmusic

Merge Risk: ⚪ Minimal · up to b43a5

This change lets the Device panel find Android SDKs and Java installed after startup, and it reports Java 17 or later as a requirement. No actionable merge-blocking risk was identified; macOS and Windows were not tested end to end.

Security architecture risk: 🔵 Low · up to b43a5

Toolchain discovery now happens when operations need it, while existing process ownership and SSH command quoting remain in place. No introduced security vulnerability was established. Risk remains limited but nonzero because installed toolchains execute with the host account’s privileges, and a running helper can retain an older environment.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Toolchain compromise could affect the executing server account or configured SSH account, rather than only one emulator, because discovered tools feed existing process execution environments. The PR changes discovery timing and fallback choices; the inspected evidence does not establish attacker write access to those locations or a new privilege transition.

Trust Boundaries and Controls

  • observed — SSH execution retains the configured target and identity file. Command arguments are individually shell-quoted, and the environment preamble plus command is quoted for the remote shell. The JDK fallback change does not replace these dispatch controls.

Resilience and Maintainability Implications

  • inferred — Keeping hub identity stable while refreshing command environments avoids unnecessary process replacement, but permits toolchain divergence during installation or replacement. This is a lifecycle limitation, not an established security concern: no changed authorization, ownership check, or cleanup boundary was found to depend on a single immutable toolchain snapshot.
Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description gives detailed problem, change, and verification information, but it omits the required Scope and approval section and does not provide a triaged issue, explicit maintainer approval, o… Add a Scope and approval section. Link the triaged issue or approval discussion with the maintainer approval comment, or explain why this focused obvious-bug fix qualifies for the exemption. Rename Validation to Verification if the reposito…
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: allowing the Device panel to find Android SDKs installed after server startup.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description gives detailed problem, change, and verification information, but it omits the required Scope and approval section and does not provide a triaged issue, explicit maintainer approval, or a clear justification for the small-fix exemption.

Resolution

Add a Scope and approval section. Link the triaged issue or approval discussion with the maintainer approval comment, or explain why this focused obvious-bug fix qualifies for the exemption. Rename Validation to Verification if the repository requires the template headings to match exactly.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant