fix(android): map adb serials to mobilecli device IDs#383
Conversation
Translate adb emulator serials to the AVD IDs expected by mobilecli for crash and screen-recording commands. Fall back to a sanitized AVD name when discovery has no match, preserve physical-device serials, and select adb.exe on Windows when ANDROID_HOME is configured. Addresses mobile-next#359.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdates ADB executable selection to use 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/server.ts (1)
166-174: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd emulator pattern guard before
getConnectedDevicesWithDetails().
getMobilecliDeviceIdcallsgetConnectedDevicesWithDetails()(which runsadb devicesplus 3adb shell getpropcalls per device) for every crash and screen-recording operation, including physical Android devices and iOS devices. SinceresolveAndroidDeviceIdreturns the original ID for non-emulator IDs, these adb calls are wasted for non-emulator devices. Adding the same regex check before the expensive call avoids unnecessary I/O on every invocation.♻️ Suggested optimization
const getMobilecliDeviceId = (deviceId: string): string => { + if (!/^emulator-\d+$/.test(deviceId)) { + return deviceId; + } const androidDevice = new AndroidDeviceManager() .getConnectedDevicesWithDetails() .find(device => device.deviceId === deviceId); return androidDevice ? mobilecli.resolveAndroidDeviceId(deviceId, androidDevice.name) : deviceId; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server.ts` around lines 166 - 174, Update getMobilecliDeviceId to detect emulator-style device IDs with the existing emulator pattern before constructing AndroidDeviceManager or calling getConnectedDevicesWithDetails. Return the original deviceId immediately for non-emulator IDs, while preserving the current lookup and resolveAndroidDeviceId behavior for matching emulator IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/server.ts`:
- Around line 166-174: Update getMobilecliDeviceId to detect emulator-style
device IDs with the existing emulator pattern before constructing
AndroidDeviceManager or calling getConnectedDevicesWithDetails. Return the
original deviceId immediately for non-emulator IDs, while preserving the current
lookup and resolveAndroidDeviceId behavior for matching emulator IDs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8807c95-6d86-420c-b273-6e1a7a56eabf
📒 Files selected for processing (5)
src/android.tssrc/mobilecli.tssrc/server.tstest/android-path.test.tstest/mobilecli.test.ts
Wait for mobilecli's screen-recording readiness signal before reporting success. Propagate process errors, early exits, and startup timeouts to the MCP caller.
|
Addressed the linked-issue warning in 0a8a828. |
Use a local adb-reversed page, handle Chrome first-run prompts, and assert display and screenshot properties that are stable across emulator configurations.
Classify adb emulator serials once and return physical Android and iOS identifiers before expensive Android device discovery.
This reverts commit 9b39ec2.
Describe the new ADB naming, emulator mapping, and recording startup functions so their contracts are explicit and automated documentation coverage remains above threshold.
Use standard multiline JSDoc with parameter and return tags so automated docstring coverage can recognize every function added by the pull request.
|
@coderabbitai The Docstring Coverage pre-merge check still reports 0.00%, but every named function and method added by this PR now has standard multiline TypeScript JSDoc with @param and @returns tags, including the nested recording callbacks. Please re-evaluate the check and list the exact functions it still considers undocumented. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd . -e ts | xargs -I{} echo {} | grep -E 'android.ts|mobilecli.ts|server.ts' || true
echo "---- android.ts ----"
rg -n "^\s*(export\s+)?(async\s+)?function|=>|/\*\*" src/android.ts | head -80
echo "---- mobilecli.ts ----"
rg -n "^\s*(export\s+)?(async\s+)?(function|class|public|private)|/\*\*" src/mobilecli.ts | head -150
echo "---- server.ts (relevant) ----"
rg -n "getMobilecliDeviceId|/\*\*" src/server.ts | head -50Length of output: 3482 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "---- android.ts lines 25-45 ----"
sed -n '25,45p' src/android.ts
echo "---- mobilecli.ts lines 55-270 ----"
sed -n '55,270p' src/mobilecli.ts
echo "---- server.ts lines 155-180 ----"
sed -n '155,180p' src/server.tsLength of output: 8542 ⚙️ CodeRabbit configuration inspected Length of output: 5318 Hi Cosm1cAC — good question. A few clarifications on how this works:
If the score is still off after the recompute, the most likely explanation is the "whole touched-file surface" scope described above rather than a parsing issue with your JSDoc — in that case, adding brief docstrings to the pre-existing methods listed above would raise the percentage. Since this check is warning-only, you can also choose to leave it as-is without blocking the merge. ✏️ Learnings added
✅ Action performedReview finished.
|
Boot an available iOS simulator on GitHub's macOS runner, verify mobilecli discovery and install the device agent before running the simulator suite. Keep the device test entry point explicit for local and CI use.
1 participant
Problem
Android MCP tools expose ADB serials such as
emulator-5554, while mobilecli identifies the same emulator by its AVD ID, such asCodex_API_36. Passing the ADB serial to crash and screen-recording commands returnsdevice not found. The recording tool also reported success before the mobilecli process confirmed startup. On Windows, the ADB path check usedprocess.env.platform, which could prevent AVD-name lookup whenANDROID_HOMEwas set.Fix
process.platformto selectadb.exeon Windows.Verification
Fixes #359