fix(deps): name the platform a package-manager command belongs to - #652
Open
filip131311 wants to merge 1 commit into
Open
fix(deps): name the platform a package-manager command belongs to#652filip131311 wants to merge 1 commit into
filip131311 wants to merge 1 commit into
Conversation
On Linux the missing-adb error told the user to run `brew install --cask android-platform-tools`. These messages are handed to an agent as remediation and it runs what they say, so an unlabelled command reads as the instruction on a machine that has no Homebrew — a wasted turn at best, and Windows had the same problem silently. Each hint now leads with a route that works anywhere — the SDK Manager, or the standalone download — and any package-manager command says which platform it is for. The brew cask stays, labelled: for the tools `adb` actually gates (a USB phone, an already-running emulator) it is still a complete one-command fix on macOS, so removing it would have traded one platform's wrong advice for another's missing advice. `sdkmanager` is qualified too, since it ships inside the SDK command-line tools and is not available to the reader who has no SDK at all. The ffmpeg hint had the same defect and gets the same treatment. It also warns that the build needs libx264: Fedora's default `ffmpeg-free` package omits it, and the encoder is hardcoded, so that install would fail later with a message about an exit code rather than a missing codec. The `xcrun` hint now leads with the requirement a reader on another platform cannot satisfy — a device id is classified by its shape, so an iOS tool can be reached from a Linux host, where "run xcode-select" was the first thing it said. Following the adb hint via `sdkmanager` left the tool still failing, because the SDK root the Homebrew command-line-tools cask uses was not among the probed locations. Added, so the remediation actually unblocks. Fixes #641
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.
Fixes #641.
Reproduced on a real Linux VM (Ubuntu aarch64, published
0.18.0, noadb):What changed, and why not the obvious thing
My first instinct — and the issue's second suggestion — was to delete the package-manager specifics
entirely. Two things killed that:
sdkmanagerships inside the SDK command-line tools. A reader with no SDK cannot run it, soit can't be the headline replacement. It's kept, but qualified with "if you already have the SDK
command-line tools".
adbgates every USB-phone andalready-running-emulator path (
launch-app,keyboard,settings-permissions,tv-remote,reinstall-app,open-urlon Android) — none of which need the emulator. For those,brew install --cask android-platform-toolsgenuinely is a complete one-command fix. Dropping itwould have traded Linux's wrong advice for macOS's missing advice.
So the rule is labelled examples, no host branching: every hint leads with a route that works
anywhere (SDK Manager, standalone download), and any package-manager command names its platform.
No
process.platformread, no mechanism, nothing new to test per host — and Windows is fixed too,which a darwin/non-darwin split would have missed.
Also in scope
ffmpeg(capture.ts) had the identical defect and is the more reachable one on Linux, whereffmpeg is the encoder for Android recording. Same treatment. It additionally warns that the build
needs libx264:
capture.tshardcodes-c:v libx264with no fallback, and Fedora's defaultffmpeg-freepackage omits it — so that install "succeeds" and then dies with an exit code and nomention of a codec.
xcrunnow leads with the disqualifier.classifyDeviceis purely shape-based, so any8-4-4-4-12 udid routes to the iOS branch on any host; a Linux reader was being told to run
xcode-select --installbefore being told iOS needs a macOS host. (Making every iOS tool failup front on a non-macOS host is the real fix for that class — it's a behaviour change with its own
failure code, so I left it out; happy to file it.)
sdkmanagerroute left the tool still failing: theHomebrew command-line-tools cask roots the SDK at
/opt/homebrew/share/android-commandlinetools,which wasn't probed. A remediation string that doesn't unblock isn't fixed.
skills/argent-screen-recording/SKILL.mdquoted the oldbrew install ffmpeg— updated so theshipped skill and the runtime error agree.
Not touched:
watermark.ts's/opt/homebrew/bin/ffmpegand the existing Android roots. Those aresearch paths — probing a Homebrew prefix on a machine without Homebrew simply misses.
Verification
On the Linux VM: the exact repro now returns the labelled hint. Then, satisfying it, the dep gate
advances to the next dependency (
emulator) instead of repeating — i.e. following the hintactually unblocks
adb, which is the only thing that makes a remediation string correct.One honest caveat from that exercise: Google's standalone Linux platform-tools zip is x86-64 only,
so on this aarch64 VM the downloaded binary can't execute. That's a limitation of the upstream
download, not of this change, and the SDK-Manager route is unaffected — but it means the
"download-and-run" leg was verified by resolution, not by executing Google's binary.
Tests: all existing assertions pass unchanged — keeping the labelled brew cask means the
/android-platform-tools/assertions incheck-deps.test.tsandhttp-dep-gate.test.tsnever neededrelaxing. Added a guard that states the actual invariant rather than banning one word: every hint that
names
brew/apt/dnf/pacmanmust also name the matching platform, every Android hint must offera platform-agnostic route and
$ANDROID_HOME, and thexcrunhint must mention the macOS-hostrequirement before
xcode-select. Banning the stringbrewwould have passedapt install adb—the same bug — while failing the correct fix.
tool-server 3092 passed.
🤖 Generated with Claude Code