Repository navigation
feat: filter mobile status/nav bars and dev overlay; viewport from snapshot - #67
Merged
cryptotavares merged 2 commits intoOct 29, 2026
Conversation
cryptotavares
force-pushed
the
cryptotavares/mmcli-p1-13-mobile-snapshot
branch
from
October 25, 2026 09:39
3cc8aa9 to
52e4d72
Compare
cryptotavares
force-pushed
the
cryptotavares/mmcli-p1-13-mobile-snapshot
branch
2 times, most recently
from
October 25, 2026 09:54
9c6fb65 to
6136dc6
Compare
This was referenced Oct 25, 2026
cryptotavares
force-pushed
the
cryptotavares/mmcli-p1-13-mobile-snapshot
branch
from
October 27, 2026 15:25
417928f to
a5259d8
Compare
cryptotavares
marked this pull request as ready for review
October 27, 2026 18:07
…apshot
Ticket P1-13.
- filterAppHierarchy (new src/platform/mobile-hierarchy.ts) runs once per
snapshot, before the a11y tree, testIds and generated locators are built,
so all of them agree:
- Android: drop top-level windows shaped like a bar (full width or
height, at most 15 % thick) whose ids are all `com.android.systemui:*`
(status bar: clock, signal, battery change on every snapshot; nav bar).
Other system windows (biometric prompt, consent dialogs, permission
dialogs, keyboard) stay. Flat hierarchies: drop only status/nav bar ids.
- iOS / Android: drop the Expo dev menu button (`gearshape.fill` plus its
"Tools" label). `MobilePlatformDriver` option `excludeDevOverlay`
(default true, new exported `MobilePlatformDriverOptions`).
- The testId viewport comes from the snapshot (iOS root frame, Android
extent of the top-level windows); getWindowSize() is called (and cached)
only when the snapshot has none. Before, every testId read made one extra
device call.
- Mobile settle: the time budget includes the first capture, and an
unchanged screen is not re-captured when the first capture already took
>= 1 s (MOBILE_SETTLE_SLOW_CAPTURE_MS). Busy screens still retry.
- Observations, describe_screen, list_testids and accessibility_snapshot
take one snapshot each (P1-08 captureScreen); tests lock that in.
cryptotavares
force-pushed
the
cryptotavares/mmcli-p1-13-mobile-snapshot
branch
from
October 28, 2026 11:29
a5259d8 to
262b778
Compare
itsyoboieltr
approved these changes
Oct 28, 2026
cryptotavares
added a commit
that referenced
this pull request
Oct 29, 2026
…refs (#64) > **Summary:** Cancel timed-out requests cooperatively and fence ref-map commits against aborted or older requests. Capture mobile trees and test IDs together, and settle mobile observations after mutations. > > **Scope change (2026-10-27):** the ref-stability layer (renumbering refs so unchanged elements keep their numbers) was removed in `6944868`. Refs are positional again, as on `main`. See [Why no ref stability](#why-no-ref-stability). **Stack context:** mm CLI v2, Phase 1, **P1-08 + P1-09**, combined in one PR. Base: [#62](#62) (`cryptotavares/mmcli-p1-06-observe-level-no-state`); followed by [#65](#65). ## Problem A queue timeout rejects the caller, but the task can keep running and overwrite another request's refs. An observation timeout also leaves collection running. Mobile observations read test IDs and the tree from separate snapshots. A click can return the old screen or `Unlock, busy`. ## Solution Keep cancellation and commit fencing at separate boundaries: 1. **Cooperative cancellation:** `RequestQueue` passes an `AbortSignal` and aborts it before timeout rejection. Tools check supported checkpoints. Observations use a linked controller and their own deadline. 2. **Commit fencing:** the server owns a small `RefStore` (94 lines). Each started task gets a generation. Every ref write (`sessionManager.setRefMap` / `clearRefMap` from tools, and the observation) checks the generation and request signal, even when a tool ignores cancellation. ```mermaid flowchart TD A[Queued task starts] --> B[Create signal and ref generation] B --> C[Run tool through guarded context] C --> D[Optional observation checks linked abort signal] C -->|ref write| F{Request current and not aborted?} D -->|ref write| F F -->|yes| G[Commit map] F -->|no| X[Drop write. Keep current refs.] T[Queue timeout aborts request] -.-> C T -.-> D T -.-> F classDef new fill:#fff3cd,stroke:#d39e00 classDef stop fill:#f8d7da,stroke:#c82333 class B,C,D,F,G,T new class X stop ``` Yellow boxes are new. Red boxes stop the ref write. Tools write refs through `context.sessionManager`, which the daemon proxies: `setRefMap` and `clearRefMap` go through the request handle. Other members retain the real receiver, including `#private` fields. `ISessionManager` stays unchanged. Optional `captureScreen()` derives both mobile outputs from one snapshot; drivers without it retain separate reads. After eligible mutations, retry busy screens and allow one unchanged-screen retry, compared against the last tree the agent received (`lastScreen`: the delivered observation, else the `describe_screen` / full `accessibility_snapshot` tree). Launch, cleanup, and non-batch tools that set refs skip settling. ## What changes | Condition or boundary | Before | After | |---|---|---| | Queued task times out | Remaining work can continue and replace refs | Abort checkpoints stop cooperative work; the commit fence drops later ref writes | | Observation reaches its own timeout | Collection continues after the response | Abort collection; the final checkpoint prevents its late ref write | | Observation not delivered (`observe none`) | Still became the diff baseline | Not recorded; the next diff is against what the agent last saw | | `list_testids` | Reads the tree and replaces refs despite returning no refs | Collect only test IDs; leave refs unchanged | | Mobile discovery and observations | Separate snapshots can disagree or show an unsettled screen | Share a snapshot when supported; bound post-mutation settle retries | Queue timeout errors use HTTP 500 with `TOOL_EXECUTION_FAILED` and `Tool execution timed out after ${effectiveTimeout}ms`. `MM_REQUEST_ABORTED` is the internal checkpoint response. Cancellation does not undo an action already dispatched to a driver. Refresh with `describe-screen` before retrying. ## Decisions for reviewers | Decision | Reason | |---|---| | Keep abort checkpoints and the commit fence | A driver operation can finish after cancellation. Ref ownership must remain safe independently. Synchronous abort-handler settlement can win the queue's timeout race. | | No ref stability; refs stay positional | See [Why no ref stability](#why-no-ref-stability). | | The observation's refMap always wins, also after a tool set refs | The observation is the last tree in the response, so every ref in it resolves. | | `lastScreen` updates only after a delivered response | A timed-out request's tree is never the settle baseline. | ## Review guide Read the files in this order: | # | File | What to check | |---|---|---| | 1 | `src/server/request-queue.ts`, `src/types/http.ts` | Abort-before-reject ordering, bounded settle wait, timeout clamp, and optional context additions. | | 2 | `src/server/ref-store.ts` | Generation and abort checks; `toolSetRefs`. | | 3 | `src/server/create-server.ts` | Proxy receiver binding (`setRefMap`, `clearRefMap`), observation cancellation checkpoints, `screenAfterResponse` / `lastScreen`, batch behavior, separate diff baseline. | | 4 | `src/tools/utils.ts`, `src/tools/interaction.ts`, `src/tools/batch.ts`, `src/tools/discovery-tools.ts` | Checkpoints and `list_testids` tree-read removal. | | 5 | `src/platform/types.ts`, `src/platform/mobile-platform-driver.ts`, `src/server/screen-settle.ts` | Optional capture boundary, shared snapshot, busy detection, and bounded retries. | | 6 | Tests beside these modules; `README.md`, `SKILL.md` | Timeout isolation, matching exceptions, snapshot consistency, and caller recovery guidance. | <details> <summary>Testing</summary> At `6944868`: `yarn test` 52 files / 1528 tests pass; coverage 97.03 / 93.95 / 95.21 / 97.19; `yarn lint` passes. Every branch above it in the stack (#65, #66, #67, #44, #68) was restacked and passes `yarn test` + `yarn lint` at its own tip. </details> ## Breaking changes None. - `ToolContext.signal`, `IPlatformDriver.captureScreen` and `ScreenCapture` are optional additions. - `RequestQueue.enqueue` now passes the signal to its callback. Existing callbacks that take no argument keep working. ## Consumer impact | Consumer | Change needed | Done on branch | |---|---|---| | Extension `test/e2e/playwright/llm-workflow/` | None. It does not use `RequestQueue`, and its `setRefMap`/`getRefMap` implementation is unchanged. | n/a | | Mobile `tests/llm-workflow/` | None (same). | n/a | ## Why no ref stability The first version of this PR also renumbered refs so an element kept its number across observations (`stabilizeRefs`, a session high-water mark, baseline checks, merging tool and observation refs: about 500 lines of code and 900 of tests). It was removed because the problem it solves is rare: - Refs are deterministic per tree: the same screen gives the same refs, and an unchanged screen diffs as empty (device proof below). - Since P1-05/P1-06, read commands collect no observation, so they no longer renumber anything. - The only case it fixed is an element inserted above the one the agent holds. The response then carries the new refs (the diff lists the shifted nodes, or the full tree when every node shifted), and SKILL.md tells agents to use the refs from the latest response. - No baseline defect (B1–B15) was a wrong-element click caused by renumbering. If real runs show agents clicking stale refs, it can come back as its own ticket. ## Proof **After the shrink (`6944868`, run on the restacked stack tip):** | Check | Extension | iOS | Android | |---|---|---|---| | Timed-out click, then `describe-screen` (queueMs 0), then click by ref | pass | pass | pass | | `describe-screen` twice: identical refs | 17/17 | 67/67 | 242/242 | | Unchanged screen, 3 taps: diff | 4,243 B, +0 −0 | 4,860 B, +0 −0 | 5,306 B, +0 −0 | | Navigate away and back, click by the latest ref | pass | pass | pass | Android navigation observations can still show the previous screen (the new screen mounts about 2 s later). That is the existing Android settle limit noted in #44, not changed here. **Earlier revision (with ref stability), kept for history:** Tested with yalc builds in the Extension (e2e build, headless) and Mobile (dev client on iOS sim and Android emulator). The first full run used an earlier revision (e804b95). The final commit got a targeted mobile recheck plus a smoke run on all three platforms. Details are in the proof file in the mm-cli-v2 workspace. | Check | Before (#62) | After | |---|---|---| | `click --testid does-not-exist --timeout 500`, then `describe-screen`, then click by ref | – | works on ext, iOS and Android (queueMs 0, intended element clicked) | | Android unlock-click observation | login screen ("Unlock, busy") in 3/3 | no login or busy in 4/4 (3 wallet screen, 1 tab bar only) | | Unchanged-screen diff, iOS (`wait-for wallet-screen`) | 7495 B | 7518 B, diff +0 −0 | | Unchanged-screen diff, Android (`click tab-bar-item-Wallet`, p50 of 3) | 7672 B, observationMs 1924 | 40–110 KB, observationMs **965–1001** (one snapshot instead of two) | | Ext bench p50 (click / get-state / list-testids totalMs) | 580 / 5 / 565 | 582 / 6 / **60** (`list_testids` no longer reads the tree) | The Android unchanged-screen diff gap in that revision is closed by #67 and #44 (see the table above).
cryptotavares
added a commit
that referenced
this pull request
Oct 29, 2026
> **Summary:** Give duplicate browser accessibility refs unique DOM selectors when possible. Resolve ref targets page-wide before checking `--within`, so a parent cannot select another member of an ambiguous group. **Stack context:** mm CLI v2, Phase 1, **P1-11**. Base: [#65](#65) (`cryptotavares/mmcli-p1-10-ext-speedups`); followed by [#67](#67). Review #65 first. ## Problem Browser refs use role-and-name selectors. Two elements with the same role and name therefore share one selector. A click fails in Playwright strict mode. With `--within`, the parent can narrow that selector to a different element and click the wrong row. The Extension has repeated labels such as “Show QR code”, “Copy address”, and “Network options”. Ref identity must distinguish these elements without assuming AX-tree order matches Playwright locator order. ```text Two rows each expose a “Show QR code” button. Ref for row B + --within row A Before: the shared role selector can select row A's button. ``` ## Solution Build ref selectors at the browser discovery boundary. Keep the existing selector map consumed by Playwright tools and session managers. ```mermaid flowchart TD A[AX snapshot candidates] --> B[Group normalized role-and-name selectors] B -->|unique role| C[Keep exact role selector with snapshot scope] B -->|duplicate role or generic| D[Build unique DOM selector in cached isolated world] D --> E{Selector result?} E -->|unique| H[Store selector in ref map] E -->|unavailable| F[Keep strict role or text fallback] E -->|closed shadow root| G[Store unreachable marker] C --> H F --> H H --> I[Resolve Playwright locator for action] G --> X[Reject ref action before locator creation] classDef new fill:#fff3cd,stroke:#d39e00 classDef stop fill:#f8d7da,stroke:#c82333 class B,C,D,E,F,G new class X stop ``` Yellow boxes are new. Red boxes stop the action. The DOM builder prefers a unique `data-testid`, then a unique `id`, then a unique ancestor-anchored path. It uses `nth-of-type` or `nth-child` only within a path whose uniqueness it checks. Open shadow roots use chained host selectors: `css=<host> >> css=<inside>`. `callOnNodeInIsolatedWorld` resolves the backend node in the cached session and isolated world introduced by **P1-10**. This keeps selector construction outside the Extension's scuttled main-world globals. Lookups run concurrently with a fixed bound. Session or world failures retry once; missing nodes and page-function exceptions do not. Role and text selectors normalize whitespace and escape quoted values. The explicit `s` flag documents the role engine's existing exact, case-sensitive behavior. Scoped snapshots prefix role and text selectors with `css=<root> >> nth=0`. Unique DOM selectors remain page-wide; the scope root itself uses the root selector. For `--within` plus a ref, `waitForTarget` checks page-wide ambiguity before containment. It rechecks an outside-parent result after a short delay to allow DOM movement. Late targets wait for attachment within the shared timeout budget. ## What changes | Condition or boundary | Before | After | |---|---|---| | Duplicate role and name with resolvable DOM nodes | Refs share a role selector | Each ref gets its own verified unique DOM selector | | Duplicate without a unique DOM selector | Ambiguous role selector | Keeps the strict fallback; adds no AX-order `nth=` guess | | Selector builder finds a closed shadow root | Other selectors cannot reach that node | Stores an unreachable marker; ref resolution rejects it | | Scoped snapshot | Role selectors search page-wide | Role and text selectors retain the snapshot root; fallback relatives remain page-wide | | Ref target with `--within` | Parent can narrow an ambiguous ref to another element | Rejects page-wide ambiguity and a confirmed outside-parent target | | Consumer interface | `Map<string, string>` and `ISessionManager` | Same contracts; browser discovery and locator resolution own the change | Ref-specific errors include `Ref e2 is not inside the within element` and `Ref e2 is inside a closed shadow root: it cannot be targeted`. An ambiguous ref reports `Ref e2 is ambiguous (9 matches); take a new snapshot or target it by testId or selector`. ## Decisions for reviewers | Decision | Reason | |---|---| | Keep selector-backed refs instead of the backend-node ref model preferred by **P1-11** | Tools need Playwright locators. Consumers already implement `ISessionManager` with selector strings. | | Verify DOM paths instead of assigning an AX-order index | Dialogs, `inert`, shadow roots, slots, and `aria-owns` can produce different AX and Playwright orders. | | Reuse the **P1-10** isolated-world cache | Avoids per-node contexts and keeps shared session recovery in `cdp.ts`. | | Treat `--within` as containment for ref targets | A parent must not choose among the page-wide matches of an ambiguous ref. | ## Review guide Read the files in this order: | # | File | What to check | |---|---|---| | 1 | `src/tools/utils/discovery.ts` | Candidate grouping, scoped snapshots, fallback selectors, and page-wide `within` checks. | | 2 | `src/tools/utils/in-page.ts` | Self-contained selector builder, uniqueness rules, shadow roots, and escaping. | | 3 | `src/tools/utils/cdp.ts` | Cached-world reuse, node resolution, retry classification, and object-group release. | | 4 | `src/tools/utils/{discovery,in-page,cdp}.test.ts` | Duplicate targeting, parser round trips, bounded concurrency, shadow cases, and containment races. | | 5 | `src/server/create-server.test.ts` | Queue-wait test holds the first request longer; discovery tests tolerate the remaining timeout budget. | | 6 | `README.md`, `SKILL.md` | Browser ref guarantees, strict failures, and `--within` guidance. | <details> <summary>Testing</summary> - Fresh check at remote head `e93a529e34cc3b1c65615e87bbd282a23e16f504`: `CI=true yarn vitest run src/tools/utils/cdp.test.ts src/tools/utils/discovery.test.ts src/tools/utils/in-page.test.ts src/server/create-server.test.ts src/platform/playwright-driver.test.ts --coverage.enabled=false --maxWorkers=2` passes: 5 files, 352 tests, no type errors. - Existing tests cover duplicate selectors (**AC1**), exact names (**AC2**), scope roots, strict fallbacks, and delayed or ambiguous `within` targets. - In-page tests execute function source in a VM with a fake DOM. They cover open, nested, slotted, and closed shadow roots, escaping, and scuttled globals. - The `Proof` and `Unit tests` sections below retain the original historical evidence. The fresh check does not repeat that live validation. </details> ## Breaking changes None. Refs are created per snapshot, so no caller can depend on the old selector strings. Behaviour changes, both towards failing instead of hitting the wrong element: - `within` + a ref outside the parent now fails fast instead of acting inside the parent; - a ref that cannot be made unique fails in strict mode, as before. ## Consumer impact | Consumer | Change needed | Done on branch | |---|---|---| | Extension `test/e2e/playwright/llm-workflow/` | None | n/a | | Mobile `tests/llm-workflow/` | None (browser-only code paths) | n/a | ## Proof Tested with a yalc build of this branch in the Extension (e2e build, headless, fixture wallet). For the "before" I built #65 (50eb396) in a temp clone and ran the same script. | Check | Before (#65) | After (f7057e7) | |---|---|---| | Click the ref of a duplicated label (9 cases: groups of 2, 9 and 13, positions 0–8) | **9/9 fail** (strict mode: 2 / 9 / 13 matches) | **9/9 hit the right element** (dialog heading, clipboard address, Edit form name and chain id) | | All 13 "Network options" refs → Edit form | strict-mode failure | 13/13 open their own network | | `--within <first row>` + ref of the Solana row's QR button | opened **Ethereum's** QR dialog | fails fast: "not inside the within element" (750 ms wall, mostly CLI startup) | | "Privacy" vs "Privacy Policy" links | correct | correct (no change; `role=` was already exact) | | Smoke (unlock by ref, tabs, send/back, account list, `within`, scoped snapshot) | 7/11 | **11/11** | | `describe-screen` toolMs p50: home / networks / receive | 514 / 4033 / 4023 | 514 / 4031 / 4027 | Generic Chromium cases (built dist, real Chromium; none of these occur on the Extension today): modal `<dialog>`, `inert`, open, nested and slotted shadow DOM, `aria-owns`, and a page whose main world makes `CSS`, `Element`, `Node`, `window` and `Array.from` throw. 39/41 refs click the right element. The 2 closed-shadow refs fail on purpose, and **none hits a wrong element**. On 40 duplicate buttons, snapshot time went from 42 to 33 ms, because lookups now run concurrently. **Pre-existing issues found (not caused by this PR, unchanged on both builds):** - The home token-row ref never clicks: Chromium exposes a link, but Playwright sees no link because the element has no `href`. - `describe-screen` takes ~4 s on networks, receive, the account list and About; a fixed timeout is suspected. <details> <summary>Unit tests</summary> ``` Test Files 51 passed (51) Tests 1673 passed (1673) Type Errors no errors All files | 97.42 | 94.48 | 96.55 | 97.57 yarn lint → exit 0 ``` New tests: - AC1: duplicates get distinct unique selectors. - AC2: "Send" vs "Send max" are exact. - Escaping, checked with a round trip through Playwright's own selector parser; skipped when the internal path is missing. - Scoped prefix; `text=` stays strict; no `nth` fallback. - `in-page` builder in a `vm` with a fake DOM: testid, then id, then path; shadow roots and host chaining; slotted `:nth-child`; closed shadow marker; scuttled globals; `CSS.escape` parity. - `callOnNodeInIsolatedWorld`: retry, node gone, exceptions. - `within`: ambiguous ref, ref not inside the parent, and the race when the element moves into the parent between checks. `41a404f` deflakes the P1-01 queue-wait timing test: under host load it saw 45 ms against a 50 ms threshold, so it now holds the first request for 250 ms. </details>
cryptotavares
added a commit
that referenced
this pull request
Oct 29, 2026
> **Summary:** Reduce iOS and Android accessibility output by omitting ancestor `path` and pruning layout-only elements. Keep named nodes, controls, progress indicators, and unlabelled coordinate targets. **Stack context:** mm CLI v2, Phase 1, **P1-14**. Base: `cryptotavares/mmcli-p1-13-mobile-snapshot`; review [#67](#67) first. Next: [#68](#68). ## Problem Mobile `path` repeats native ancestor class names, such as `android.widget.FrameLayout` and `RCTViewComponentView`. These names add no ref or testId targeting information. Unnamed layout wrappers further inflate `describe-screen` and observation payloads. Browser ancestry has a different purpose: it holds dialog and heading context used by row collapse. Shared comparison code must accept mobile nodes without losing that browser context. On iOS, idb also varies frame key order. Equivalent frames can therefore produce different serialized screen signatures. ## Solution Apply pruning at the `MobilePlatformDriver` normalization boundary, after the app-hierarchy filtering introduced by P1-13. ```mermaid flowchart TD A[App-filtered mobile snapshot] --> B[Prune layout-only elements; hoist kept descendants] B --> C[Keep unlabelled RN leaves at least 24 x 24] C --> D[Flatten in pre-order; number kept nodes] D --> E[Omit path; write bounds in fixed key order] E --> F[Build refMap for testId, label, or value] F --> G[Mark duplicate stable identifiers as ambiguous] A --> H[Read test IDs and locators from app-filtered hierarchy] classDef new fill:#fff3cd,stroke:#d39e00 class B,C,D,E new ``` Yellow boxes are new. `pruneLayoutOnly` keeps elements with a label, value, identifier, actionable type, or progress-indicator type. It matches exact iOS AX types and Android class-name suffixes. Removed wrappers give way to their kept descendants, in order. The function preserves input objects and returns unchanged subtrees by reference. A plain React Native view also stays when it has no kept children and measures at least 24×24. This retains unlabelled icon buttons, including the iOS wallet action buttons. These nodes expose `bounds` for `tap-coordinates`. Both `getAccessibilityTree` and `captureScreen` use this normalization. Test IDs and generated locators still use the app-filtered hierarchy before layout pruning. Browser nodes retain their `path`. Shared diffing and row collapse treat two absent paths as equal, but distinguish absence from an empty array. The filter and screen-settle logic share `MOBILE_BUSY_TYPE`, so pruning retains the progress indicators that settling detects. Normalization writes bounds in `x, y, width, height` order to stabilize screen signatures. ## What changes | Condition or boundary | Before | After | |---|---|---| | iOS and Android accessibility trees | Include layout wrappers and every ancestor role in `path` | Omit `path`; flatten kept nodes in pre-order | | Named or identified nodes, controls, and progress indicators | Listed among layout wrappers | Stay listed; refs number only kept nodes | | Unlabelled plain RN views | All sizes appear | Keep leaves after pruning when both dimensions are at least 24; use their bounds for coordinate taps | | Ref targeting | Only testId, label, or value creates a `refMap` entry | Same rule; a displayed ref alone does not make an unnamed node targetable | | Optional ancestry | Shared comparisons require path arrays | Accept absent mobile paths; preserve browser ancestry and option-summary paths when present | | iOS bounds serialization | Uses backend frame key order | Always emits `x, y, width, height` | The original Android-only change (`3bbda5c`) now covers both mobile platforms under **P1-14**. The protected proof below records payload reductions and the remaining size-target gaps. ## Decisions for reviewers | Decision | Reason | |---|---| | Prune in the mobile driver | Discovery and observations share one rule. Test ID collection and locator generation keep their existing input. | | Use platform-specific control types and a minimum-size RN leaf rule | Snapshots have no `clickable` flag. The leaf rule retains unlabelled wallet action buttons while removing wrappers. | | Keep listed nodes separate from targetable refs | Progress indicators, unnamed controls, and unlabelled views still provide useful screen or coordinate information. Existing stable-identifier targeting stays intact. | | Make the shared `path` field optional | Mobile drops native ancestry. Browser dialog and heading context remains available to row collapse. | ## Review guide Read the files in this order: | # | File | What to check | |---|---|---| | 1 | `src/tools/types/discovery.ts`, `src/tools/utils/constants.ts` | Optional public `path`, shared progress-indicator matching, and the minimum tap-target size. | | 2 | `src/platform/mobile-hierarchy.ts` | Control classification, descendant order, the RN leaf exception, and unchanged-subtree identity. | | 3 | `src/platform/mobile-platform-driver.ts` | Both normalization entry points, sequential refs, targeting-map eligibility, ambiguity, and ordered bounds. | | 4 | `src/server/observation-compaction.ts`, `src/server/screen-settle.ts`, `src/tools/utils/discovery.ts` | Optional-path comparisons, option summaries, shared busy types, and retained browser ancestry. | | 5 | `src/platform/mobile-hierarchy.test.ts`, `src/platform/mobile-platform-driver.test.ts`, `src/platform/__fixtures__/ios-wallet-home-preorder.json` | Android login and iOS wallet fixtures, controls, unlabelled action bounds, ref eligibility, and input preservation. | | 6 | Server and discovery tests; `README.md`, `SKILL.md` | Same refs across wrapper changes (`mobile-platform-driver.test.ts`), absent-path diffs, browser paths, and mobile targeting guidance. | <details> <summary>Testing</summary> Fresh checks at remote head `f38073e4ad5732ade9c22d5f6e486753a44b6416`: ```sh CI=true yarn vitest run src/platform/mobile-hierarchy.test.ts src/platform/mobile-platform-driver.test.ts src/server/observation-compaction.test.ts src/server/screen-settle.test.ts src/server/create-server.test.ts src/tools/utils/discovery.test.ts --coverage.enabled=false --maxWorkers=2 ``` - Passes: 7 files, 506 tests. The server suite emits the expected readonly-log `EACCES` diagnostic. - `yarn lint` passes. - `yarn tsc --noEmit --project tsconfig.build.json` passes for production source. - `yarn tsc --noEmit --project tsconfig.json` fails with 11 existing test-file diagnostics at this unchanged head. They include mock typing, top-level `await`, and a missing `ToolContext` import. - The `Proof` and `Unit tests` sections below retain historical evidence. The fresh checks above cover focused tests, lint, and production types. </details> ## Breaking changes - `A11yNodeTrimmed.path` is optional and absent on mobile nodes. - Mobile trees no longer include unnamed layout nodes, so mobile ref numbers change. Refs to unnamed nodes never resolved anyway. ## Consumer impact | Consumer | Change needed | Done on branch | |---|---|---| | Extension `test/e2e/playwright/llm-workflow/` | None (nothing reads `node.path`) | n/a | | Mobile `tests/llm-workflow/` | None (nothing reads `node.path`) | n/a | ## Proof Tested with yalc builds in Mobile (dev client) on the iOS simulator (idb nested format, MetaMask/device-mcp#38) and the Android emulator (MetaMask/device-mcp#39). Before = the #67 build. Byte counts are for the pretty JSON the CLI prints, with compact JSON in brackets. | Check | Before (#67) | After (f38073e) | |---|---|---| | iOS wallet home `describe-screen` | 286,526 B, 177 nodes, 6,613 `path` entries | **25,353 B** (14,933 compact), 67 nodes, 0 | | iOS wallet home buttons / tabs / inputs | 21 | 21 (0 missing); the 4 unlabelled action buttons are listed with bounds | | Android wallet home `describe-screen` | 987,001 B, 625 nodes, 21,658 `path` entries | **84,271 B** (49,772 compact), 0 | | Android wallet home actionables (incl. every testId) | 144 | 144 (0 missing) | | Android click observation: unchanged screen / screen change / full | 7,846 / 14,077 / 1,031,678 B | 7,849 / 8,047 / **74,963 B** (39,690 compact) | | iOS unchanged-screen click diff, 5 runs | varied by up to 17 nodes | empty 5/5 (bounds key order fixed) | | Click by ref, 3 targets per platform | – | iOS 3/3; Android 3/3 | | Tap a listed action button (Receive) via `tap-coordinates` at its bounds | – | opens the receive screen | **Acceptance criteria not fully met:** - iOS `describe-screen` is ≤ 15 kB only as compact JSON. The CLI pretty-prints with a 2-space indent, which is ~41 % of the bytes. - The Android **full** observation (first click on a screen) is 75 kB, against a 20 kB target. Diffs on later clicks are ~8 kB. The remaining size is named content (token rows, banners, prices) and `bounds` (~30 % of mobile bytes). Possible next steps: compact CLI output, dropping or rounding `bounds`, and a cap on long lists. **Known issues not caused by this PR:** - device-mcp's `findElement` accepts a substring testId match and takes the first one, so `click --testid account-picker` on Android taps `account-picker-avatar`. - The Android unlock click observation still shows "Unlock, busy" under host load. <details> <summary>Unit tests</summary> ``` Test Files 52 passed (52) Tests 1803 passed (1803) Type Errors no errors All files | 97.51 | 94.88 | 96.73 | 97.66 yarn lint → exit 0 ``` New tests: - `pruneLayoutOnly`, `isLayoutOnly`, `isActionableElement`: iOS exact types, `XCUIElementType` prefix not actionable, Android suffixes incl. `CheckedTextView` / `RatingBar`, progress indicators, unlabelled leaf ≥ 24×24. - Real iOS wallet-home fixture: 183 → 67 nodes; all buttons and tabs kept; the 4 action buttons listed with bounds. - Real Android login fixture. - No `path` on mobile (iOS and Android); browser `path` kept; diff and collapse with absent `path`. - Bounds key order; shared busy regex. </details>
cryptotavares
added a commit
that referenced
this pull request
Oct 29, 2026
> **Summary:** Upgrade `@metamask/device-mcp` to `^0.6.0`. 0.6.0 gives correct iOS trees (nested idb format) and fast, correct Android snapshots on busy screens (signed helper first). #44 and #67 were measured with these changes. **Stack context:** mm CLI v2, Phase 1, **P1-30**. Base: [#68](#68), the top of the stack; review that first. ## Problem Core pins `@metamask/device-mcp` `^0.5.0` (#58). On 0.x a caret range does not pick up 0.6.0. Without 0.6.0: - **iOS:** device-mcp cannot read the nested `idb ui describe-all` format, so the element tree loses its hierarchy ([device-mcp#38](MetaMask/device-mcp#38)). - **Android:** snapshots start with `uiautomator dump`. On busy screens this waits about 11 s for an idle state, then fails with "could not get idle state" ([device-mcp#39](MetaMask/device-mcp#39)). #44 (flat mobile tree) and #67 (mobile snapshot filtering) were measured against yalc builds of those two device-mcp PRs. ## Solution Adopt the device-mcp 0.6.0 release through the dependency range and the lockfile. Add one README sentence about the new Android helper install. ```mermaid flowchart TD A[Mobile describe / observation] --> B[Core MobilePlatformDriver] B --> C{device-mcp 0.6.0 snapshot} C -->|iOS| D[idb describe-all, nested format read] C -->|Android default helper-first| E[Signed helper APK, no idle wait] E -->|non-trust failure| F[1 uiautomator dump attempt] D --> G[Core flat tree, refs, testIds] E --> G F --> G classDef new fill:#fff3cd,stroke:#d39e00 class D,E,F new ``` Yellow boxes are new in 0.6.0. ## What changes | Boundary | Before | After | |---|---|---| | `package.json` dependency range | `@metamask/device-mcp ^0.5.0` | `^0.6.0` | | `yarn.lock` resolution | npm 0.5.0 | npm 0.6.0 (dependencies unchanged) | | Android snapshot on the consumer's ADB backend | `uiautomator dump` first | Signed test-only helper first, installed on first use. `DEVICE_MCP_ADB_SNAPSHOT=dump` restores the old path | | `README.md` | No mention of the helper | One sentence: helper install, first snapshot slower, how to opt out | ## Decisions for reviewers | Decision | Reason | |---|---| | New PR on top of the stack instead of amending #58 | Amending #58 would rebase the whole stack again. 0.6.0 was not released when the stack was built. | | `chore:`, not breaking | Every symbol core imports from device-mcp is unchanged. `SnapshotResult` only gains optional fields (`source`, `captureMs`), and core reads only `hierarchy`. The new Android default is a dependency runtime change on the consumer's backend, not a change to core's API. | | Core does not set `snapshotMode` | The consumer creates the `DeviceBackend` (Mobile uses `AdbBackend` directly). Core only documents the default and the opt-out. | ## Review guide | # | File | What to check | |---|---|---| | 1 | `package.json` | The production dependency range is `^0.6.0`. | | 2 | `yarn.lock` | Only the device-mcp entry and the workspace range change. | | 3 | `README.md` | The helper sentence in the mobile snapshot paragraph. | <details> <summary>Testing</summary> **Unit and lint** (at afaf75c, device-mcp 0.6.0 from npm, no yalc): `yarn build` OK, `yarn test` 51 files / 1793 tests pass, `yarn lint` passes (including the dedupe and constraints checks). **Device proof:** Mobile with this branch's core (yalc) and **device-mcp 0.6.0 from npm** (no yalc device-mcp). Real-funds wallets; only read actions, tab bar Home/Explore, account list open/back and settings list (no row opened). | Check | Extension | iOS | Android | |---|---|---|---| | Smoke: launch, unlock, describe, list-testids, get-state, cdp, screenshot | PASS | PASS | PASS | | A timed-out click does not block the next call; click by ref after it | n/a | PASS | PASS | | Two describes give identical refs | n/a | 67 / 67 | 242 / 242 | | Unchanged screen diff | n/a | +0 −0 | +0 −0 | | Ref from the latest describe clicks the intended element | n/a | PASS | PASS | Node counts are identical to the yalc device-mcp measurements used for #44 (`measure-tree.sh`): | Screen | iOS nodes | Android nodes | |---|---|---| | Wallet home | 67 → 67 | 242 → 242 | | Account list | 35 → 35 | 33 → 33 | | Settings | 26 → 26 | 65 → 65 | **Android helper path:** the emulator had helper 0.5.0. On the first snapshot, device-mcp 0.6.0 installed its bundled, signed 0.6.0 helper, and the signer check passed. During the smoke run, logcat shows 106 helper runs and 0 `uiautomator` runs. **Known and unchanged:** on Android, 3 of 4 navigation observations still show the previous screen (same as before this bump). That is tracked separately. </details>
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.
Stack context: mm CLI v2, Phase 1, P1-13. Base:
cryptotavares/mmcli-p1-11-exact-refs; stacked on #66, which reviewers should read first. Reuses P1-08's sharedcaptureScreenand refines P1-09's mobile settle policy.Problem
Mobile snapshots include elements outside the app. Android's status bar contributes changing clock, signal, and battery data. The Expo DevLauncher button also appears in app discovery results. These elements add noise to accessibility diffs, test IDs, and locator suggestions.
Each test ID read also calls
getWindowSize(), although snapshot frames can supply the viewport. The server starts its settle budget after the first capture, then can retry an unchanged screen despite a slow capture.Solution
Put app filtering and snapshot-derived viewport handling in
MobilePlatformDriver, before shared tools consume screen data. Keep settle decisions in the server.flowchart TD A[One device snapshot] --> B[Read viewport from unfiltered root frames] A --> C[Filter Android bar windows and optional dev overlay] B --> D[Build tree, refs, test IDs, or locators] C --> D D --> E[For mutating or batch observations, apply bounded settle] E -->|retry allowed| A E -->|done| F[Return final observation] classDef new fill:#fff3cd,stroke:#d39e00 class B,C,E newYellow boxes are new.
#snapshotAppsupplies one filtered hierarchy to accessibility normalization, test ID collection, and locator generation.captureScreenretains that hierarchy for its test ID reader. The existing shared-capture boundary remains intact.Android filtering requires both a bar-shaped window and at least one identifier, all from
com.android.systemui. The frame must span the viewport width or height and be at most 15% of its own length thick. This keeps non-bar System UI windows, including biometric prompts. Flat hierarchies remove only status/navigation bar identifier prefixes.The default overlay filter removes iOS
gearshape.filland its followingStaticTextlabel,Tools. On Android, it removes aToolsImageViewinside a Compose view. Empty, unnamed containers that held only removed elements disappear too. Filtering leaves the input unchanged and reuses unchanged subtrees.What changes
getWindowSize()Decisions for reviewers
MobilePlatformDriverOptionsexcludeDevOverlay: falsekeeps the dev button. Android bar filtering still applies.Review guide
Read the files in this order:
src/platform/mobile-hierarchy.tssrc/platform/mobile-platform-driver.ts#snapshotApp, shared consumers, fallback caching, and the optional constructor argument.src/server/create-server.ts,src/tools/utils/constants.tssnapshotMobile: first-capture budget, slow unchanged capture, and busy-screen precedence.src/platform/index.ts,src/index.tsMobilePlatformDriverOptionstype exports.src/platform/mobile-hierarchy.test.ts,src/platform/mobile-platform-driver.test.ts,src/server/create-server.test.tsREADME.md,SKILL.mdTesting
6136dc6e39398cc06ce2a3dcc4a0eda4280f6812:CI=true yarn vitest run src/platform/mobile-hierarchy.test.ts src/platform/mobile-platform-driver.test.ts src/server/create-server.test.ts src/tools/discovery-tools.test.ts --coverage.enabled=false --maxWorkers=2 --minWorkers=1passes: 4 files, 301 tests. Vitest reports no type errors.yarn lintpasses at that remote head.yarn tsc --project tsconfig.test.json --noEmitcheck reports 11 existing test-project diagnostics. A compiler-API comparison confirms the same diagnostics at the remote head.ProofandUnit testssections below retain historical evidence. These fresh checks do not repeat live device validation or the full suite.Breaking changes
None. The constructor option is optional. Mobile testIds and trees no longer contain Android status/nav bar windows or the Expo dev menu button.
Consumer impact
test/e2e/playwright/llm-workflow/tests/llm-workflow/Proof
Tested with yalc builds in Mobile (dev client) on the iOS simulator and the Android emulator. Before = #66 build, after = this branch. Click and benchmark rows are 5 runs each; host load was 5–14.
com.android.systemuidescribe-screenroot windows / systemui nodesgenerate-locatorson loginobservationMsp50 / p95gearshape.fill)observationMsp50 / p95list-testids/describe-screentoolMs p50idb describe(window size) per click / describe / listAC3 ("Android click
observationMsroughly halves") is not met, and core cannot meet it.waitForElement+tapElement, in device-mcp / the Mobile agent-device helper) and 1 for the observation.Still noisy on Android:
ViewGroups. P1-14 (drop layout-only nodes) covers this.Toolbar/FloatingBarHostingViewnodes remain. They are most likely UIKit's floating bar per navigation controller, not the dev launcher. They are kept for now, pending a check on a release build.Unit tests
New tests:
mobile-hierarchy.test.ts:excludeDevOverlayoption, empty-container pruning;src/platform/__fixtures__/, excluded from the build), which reduces to 1 root and 0 systemui testIds.getWindowSizeat most once,generateLocatorsfiltered.