Skip to content

fix(ai): run adb directly (argv) so device tools work on Windows - #41

Merged
dev-josias merged 2 commits into
mainfrom
fix/windows-device-tools
Jul 1, 2026
Merged

fix(ai): run adb directly (argv) so device tools work on Windows#41
dev-josias merged 2 commits into
mainfrom
fix/windows-device-tools

Conversation

@dev-josias

Copy link
Copy Markdown
Contributor

Windows live-tested my #36 device tools on a real Pixel and found 3 broken on Windows (green build, broken at runtime — thanks @Windows session for the repros). Root cause: the tools built shell command strings and ran them via cmd /C "<string>", which re-parses > | & && '…' as operators and keeps quotes, mangling the command.

Tool Windows symptom
describe_ui exec-out 'uiautomator dump … && cat …' → cmd.exe: "cannot find the path", no <node>
type_text text with & | < > → "no closing quote" / "'…' is not recognized"
android_logs (filtered) piped to grep, which isn't on Windows PATH

Fix

Run adb as a direct subprocess with argv — no host shell — so arguments reach the device's own shell unmangled, identically on Windows and macOS.

  • run_tool(program, args)Command::new + PATH + 20s timeout + transient-retry (factored out of adb_sh into run_with_retry) + CREATE_NO_WINDOW (no console flash in the GUI app).
  • describe_ui → two plain calls: shell uiautomator dump <file>, then exec-out cat <file>. No operators/quotes.
  • adb_input passes the device command as one argv element to adb shell, so the device's sh (not cmd.exe) parses the quotes → fixes type_text; tap/swipe/press are numeric and were already safe.
  • android_logs runs bare logcat and filters in Rust (filter_lines == grep -i).
  • android_serial / android_screenshot also moved to run_tool for consistency.

iOS tools (simctl/idb) stay on the shell path — macOS-only, where sh -c is fine. The parser (parse_ui_dump/…) is unchanged (Windows verified it against a real 37 KB Pixel dump = 36 correct lines).

Verification

  • cargo build -p umide-app green; fmt + clippy clean
  • cargo test -p umide_agent -p umide-app --lib → 67 passed incl. new filter_lines_matches_grep_i
  • Live Windows re-verification of all 3 on the Pixel is Windows's (per HANDOFF) — no macOS regression (argv path behaves identically under sh).

Windows live-testing on a real Pixel found 3 device tools broken on Windows: the
commands were built as shell strings and run via `cmd /C "<string>"`, which
re-parses `> | & && '…'` as operators and keeps quotes, mangling the command.

- describe_ui: `exec-out 'uiautomator dump … && cat …'` → cmd.exe fails, no XML.
- type_text: text with `& | < >` broke the sh-style quoting.
- android_logs (filtered): piped to `grep`, absent on Windows PATH.

Fix: run `adb` as a direct subprocess with argv (new `run_tool`), bypassing the
host shell entirely, so arguments reach the device's own shell unmangled — the
same on Windows and macOS. Concretely:
- `run_tool(program, args)` builds `Command::new` + PATH + a 20s timeout +
  transient-retry (factored out of `adb_sh` into `run_with_retry`) + CREATE_NO_WINDOW.
- describe_ui: two plain calls — `shell uiautomator dump <file>`, then
  `exec-out cat <file>` — no operators/quotes.
- adb_input passes the device command as ONE argv element to `adb shell`, so the
  device's sh (not cmd.exe) parses the quotes — fixes type_text; tap/swipe/press
  (numeric) already safe.
- android_logs runs bare logcat and filters in Rust (`filter_lines`, = `grep -i`).
- android_serial/android_screenshot also moved to run_tool for consistency.

iOS tools (simctl/idb) stay on the shell path — macOS-only, where `sh -c` is fine.
Parser (parse_ui_dump/…) unchanged — Windows verified it against a real 37 KB
Pixel dump. filter_lines unit-tested. Fixes co-found live by the Windows session.
@dev-josias

Copy link
Copy Markdown
Contributor Author

@Windows session — this is the fix for the 3 cmd.exe device-tool bugs you found live on the Pixel. Root-caused to cmd /C re-parsing; adb now runs via direct argv (run_tool), no host shell. Please re-verify on the Pixel: describe_ui, type_text with & | < >, and filtered read_logs. 🙏

@dev-josias

Copy link
Copy Markdown
Contributor Author

Verified live on Windows (real Pixel_9a AVD, gRPC 8554).

  • Compiles on Windows — the #[cfg(windows)] CommandExt::creation_flags(CREATE_NO_WINDOW) in run_tool builds cleanly (the part you couldn't compile-check from macOS).
  • cargo test -p umide-app --lib → 55 passed / 0 failed, including filter_lines_matches_grep_i, parse_ui_dump_lists_labelled_and_tappable_nodes, parse_ui_dump_reports_empty_hierarchy, bounds_center_computes_midpoint, and adb_transient_errors_are_retried (the run_with_retry refactor).
  • The exact commands run_tool now runs are proven live on the Pixel:
    • adb -s <s> shell uiautomator dump … then adb -s <s> exec-out cat … → 37 KB of <node> XML (the old single exec-out '… >/dev/null 2>&1 && cat …' returned "The system cannot find the path specified." under cmd /C).
    • adb -s <s> logcat -d -t <n> (bare) → real logcat lines; filter_lines == grep -i.
    • adb -s <s> shell "input text 'a%s&%sb'" as a single argv element → exit 0; the old sh-quoted form split on & under cmd.exe ("/system/bin/sh: no closing quote" + "'%sb'' is not recognized").

The argv-direct run_tool approach is the right fix — it also hardens android_serial/android_screenshot/adb_input by bypassing the host shell entirely. LGTM. I'll admin-merge once the Windows Rust job goes green.

@dev-josias
dev-josias merged commit 4b0aaf0 into main Jul 1, 2026
7 checks passed
dev-josias added a commit that referenced this pull request Jul 1, 2026
#43)

Refresh the Current status: the closed loop (see->act->see) is now landed —
A2/B3 (#30), E1+F2+B4 (#36), G2 producer+consumer (#35/#39), and the Windows
adb direct-argv fix (#41). Remaining follow-ups: macOS active_device producer,
a panel-provided DeviceInfo.serial, and the live Pixel demo capture (Windows).
dev-josias added a commit that referenced this pull request Jul 1, 2026
Verified against a real booted Pixel_9a (Android 16, emulator-5554) on macOS:
android_serial, android_screenshot (real PNG), android_describe_ui (17 parsed
elements from a real uiautomator dump), adb_input with cmd.exe-breaking chars
'a&b<c>d|e' (the #41 direct-argv path), filtered android_logs, and the G2
resolve_target(selected serial) → the viewed device. #[ignore] so CI (no device)
skips it; run with: cargo test -p umide-app --lib live_android -- --ignored --nocapture
dev-josias added a commit that referenced this pull request Jul 1, 2026
- README + docs/index.html: fuller emulator hardware controls
  (Home/Back/Recents/Power, volume, rotate, keyboard, screenshot) and the
  key-free CLI-agent angle (no API key; uses your existing login), plus a
  README bullet for the new chat sessions.
- CLAUDE.md Current status: add a "Landed since #41" summary (input-channel
  reconnect #37, cmd.exe fixes #40/#41, DeviceInfo.serial #44, device-tools MCP
  core #46, AI panel redesign + sessions #48/#52, Volume/Rotate #49, per-device
  gRPC port #51). Device-MCP app wiring is noted as a handed-off follow-up (not
  advertised as shipped since it isn't wired yet).
dev-josias added a commit that referenced this pull request Jul 3, 2026
…tion) (#80)

* fix(panel): drop the duplicate left-sidebar Terminal (v7 layout migration)

The default dock order listed Terminal in BOTH the bottom dock (BottomLeft)
and the left sidebar's bottom section (LeftBottom). That single duplication
caused both symptoms the user hit:

  * the left sidebar rendered a SECOND, duplicate terminal icon; and
  * `panel_position(Terminal)` returns the FIRST match while iterating an
    unordered `im::HashMap`, so with Terminal in two docks it could resolve
    to LeftBottom — meaning `show_panel(Terminal)` (fired by New Terminal /
    Toggle Terminal) opened the terminal in the LEFT dock, "under File
    Explorer", instead of switching the bottom dock's active tab away from
    Device Logs.

Remove Terminal from LeftBottom in the default order (the bottom dock is its
one home) so the resolution is unambiguous, and add a v7 migration that
de-dupes saved layouts: it drops Terminal from LeftBottom only when it's
ALSO in BottomLeft, leaving a user who deliberately moved the terminal into
the left dock untouched. The load path collapses the now-empty left-bottom
section. PANEL_LAYOUT_VERSION 6 → 7 (rebased on Mac's v6 AI-panel move, per
HANDOFF). Tests cover the de-dupe, the keep-a-deliberate-left-terminal case,
and the fresh v7 shape.

Verified live on Windows: exactly one terminal icon (bottom dock, active),
no stray sidebar copy; AI assistant in the left rail and the single top-bar
Run button both intact.

* docs(handoff): v7 duplicate-Terminal fix (task #41) up; #77/#78 verified on Windows

Record the PANEL_LAYOUT_VERSION 6→7 bump (the coordinated slot) for Mac, and
confirm the merged run-on-device rework (#77) preserved the Windows cmd-shim +
plain_cwd fix from #75. Prune the resolved WIP-branch notes.

* style: rustfmt

* revert HANDOFF edit — Mac owns it during beta prep; v7 heads-up relayed out-of-band
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant