[cryptotavares]

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 (cryptotavares/mmcli-p1-10-ext-speedups); followed by #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.

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.

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

Loading

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.

Testing

• 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.

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.

Unit tests

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.