fix(android): fail doctor when adb is the Windows binary on a POSIX host - #3157
Conversation
Under WSL, Windows adb.exe passes adb version but resolves host paths as Windows paths, so recording/pull/install break. Read adb's Installed as line and fail the toolchain check with reason android_adb_windows_binary_on_posix_host plus a Linux platform-tools hint.
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The environment-specific WSL behavior has unit coverage but lacks validation on an actual WSL host.
Review effort: Balanced
Findings: None
What changed in this PR
Adds detection for Windows adb.exe binaries running on POSIX hosts, preventing misleading Android doctor passes under WSL.
Changes:
- Parses full
adb versionoutput and reports a typed failure for Windows paths on POSIX hosts. - Injects the host platform and adds regression coverage.
- Documents the WSL platform-tools limitation.
| File | Description |
|---|---|
website/docs/docs/known-limitations.md |
Documents required Linux platform-tools under WSL. |
src/platform-runtime-host-diagnostics.ts |
Supplies the host platform to Android diagnostics. |
packages/provision-kit/src/toolchain-probe.ts |
Adds successful command stdout retrieval. |
packages/platform-android/src/doctor.ts |
Detects and reports incompatible Windows adb binaries. |
packages/platform-android/src/__tests__/doctor.test.ts |
Covers Windows and Linux adb combinations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
This PR is ready at dbc0161. Not blocking, and you can take or leave these: the hint at https://github.com/callstack/agent-device/blob/dbc0161/packages/platform-android/src/doctor.ts#L204 says "Under WSL" but fires for every non-win32 host, so it could name the invariant (adb must be a native binary for this host) and mention WSL as the common case. Also, when the "Installed as" line is missing (older adb), https://github.com/callstack/agent-device/blob/dbc0161/packages/platform-android/src/doctor.ts#L41 returns undefined and the check passes as before. You could optionally treat a "Running on Windows" line on a non-win32 host as the same signal. On the open review threads, the P3 thread about the WSL-specific hint still applies (#3157 (comment)). Two P2 threads do not apply, so please resolve them. The "#!/bin/sh" fixtures match sibling tests in the package, and no CI job runs on Windows (#3157 (comment)). The missing "Installed as" case is fail-open with no false failure, and the fallback is covered in the note above (#3157 (comment)). The failing check is the iOS simulator Smoke Tests job, where an |
thymikee
added
the
ready-for-human
…ut Installed as The hint fired on every non-win32 host but only advised WSL users; state the invariant that adb must be a native binary for this host and keep WSL as the example. Treat a "Running on Windows" banner as the same signal so builds that omit "Installed as" fail closed instead of passing green, and key the check on detectedVia.
|
Both non-blocking points are in 90193ef:
All three review threads are resolved. The iOS Smoke Tests lane is untouched by this change; re-running it before merge as noted. |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
Reviewed 90193ef: the code is ready for human review. The earlier notes from dbc0161 are fixed, and the new Not blocking: the open Cubic thread on known-limitations.md still holds. A mounted volume alone cannot run adb.exe on macOS or Linux. Could you say "through WSL interop or Wine" there, and drop the same phrase from the doc comment on Smoke Tests were still running at review time. This change only touches the Android doctor probe, so I do not expect it to affect them. There are no conflicts. I did not run the tests or a live WSL adb. |
|
I checked d40fd85. The only new commit changes one doc comment in Not blocking: the doc sentence now says a Windows |
Summary
From e2e feedback: under WSL with a Windows
adb.exeon PATH (e.g.ANDROID_HOMEunder/mnt/c),adb versionsucceeds, so doctor passed, but recordings, pulls, and installs failed because adb resolves host paths as Windows paths.agent-device doctornow reads adb'sInstalled asline and fails the Android toolchain check with reasonandroid_adb_windows_binary_on_posix_host(plusadbPathevidence and a Linux platform-tools hint) when a non-Windows host gets a drive-letter or UNC path. The host platform is injected from the root composition, keepingplatform-androidfree of ambientprocessauthority. Known-limitations docs gain a WSL note.5 files touched.
Validation
Tested commit: dbc01611fa4da0fa776f2ed4a48dda0b1b7fd1ae (on upstream/main 9a245d0).
pnpm vitest run packages/platform-android/src/__tests__/doctor.test.ts: 7 passed (Windows adb on linux fails with typed reason; Linux adb under WSL passes; Windows adb on win32 passes)pnpm check:affected --run --base upstream/main: all runnable checks passed (370 files / 2402 related tests). First run caught a layering violation (process.platformin platform-android), fixed by injecting the platform.Risks: no WSL host available, so validation is unit-level only. This adds the diagnosis; recording with a Windows adb is still unsupported.