refactor: declare selector resolution policy as data

[thymikee]

Part of #1630 — the policy matrix and its wiring. The remaining criterion (mutating find must not chain two resolution engines) is split out as #1654, with the reasoning in that issue.

What

Five native consumers of "resolve a selector against the screen" each hand-declared their ambiguity contract as inline requireUnique / disambiguateAmbiguous literals, so the repo's real policy matrix was only discoverable by reading four files and diffing them in your head. SELECTOR_RESOLUTION_POLICIES (packages/selectors/src/internal/resolution-policy.ts) now declares one row per caller, and selectorResolutionKnobs turns a row into the engine knobs it stands for.

| row | caller | ambiguity | rect |---|
| act | click/press/fill/focus/longPress/drag/scroll | disambiguate | ✓ |
| actCoveredDiagnosis | the post-miss "covered?" probe | first-match | ✓ |
| readText | get text | disambiguate | — |
| readUnique | is non-exists, get attrs | fail-closed | — |
| readAny | exists, find read actions | first-match | — |
| wait | wait | first-match | — |
| findAct | mutating find | reject-candidates | ✓ |

Zero ambiguity literals remain in src. The matrix declares only the ambiguity contract and the rect requirement — the fields it actually enforces; the structural pipeline stages stay with their callers and are tracked in #1656 (an earlier revision declared them as columns nothing consumed). Semantics are unchanged by construction — each row was read off its call site; the matrix names what was previously implicit.

reject-candidates is declaration-only and rejected by selectorResolutionKnobs at the type level: mutating find enforces #1625's contract through its own narrowing logic, not through engine knobs, and the types now prevent anyone wiring it up as if it were a knob.

The matrix is gate-tested, not trusted

resolution-policy-parity.test.ts follows ADR 0011's declared-plus-gate-tested pattern, because a matrix that merely claims things about callers rots silently:

• knobs must match the ambiguity contract each row names;
• every claimed structural column must appear in that caller's source (occlusion/off-screen/promotion/poll each have marker sets);
• the inverse direction too — read and wait pipelines must genuinely lack the machinery they disclaim, so the matrix can't under-report;
• no caller may reintroduce an inline requireUnique/disambiguateAmbiguous literal.

Verified revert-sensitive (not assumed): flipping readUnique to disambiguate fails the asymmetry test, and faking wait's occlusion column fails the structural test; restoring passes 7/7.

Out of scope, unchanged

Per the issue: the Maestro engine stays fully separate (ADR 0015), and whether click/tap/is gain an implicit lookup budget remains the open product decision — this just makes it a one-row change if it's ever taken.

Verification

• Typecheck, lint, format, check:layering, fallow audit + production-exports: green.
• src/commands + src/daemon: 258 files / 2,220 tests green; interaction suites 173 green. (An earlier run hit the documented subprocess-stub contention flake in an unrelated Android runtime-hints file — passes 15/15 solo and on rerun.)
• Rebased onto current main (d2f28d790) before opening.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.99 MB	1.99 MB	+2.0 kB
JS gzip	635.8 kB	636.4 kB	+558 B
npm tarball	769.2 kB	769.8 kB	+553 B
npm unpacked	2.69 MB	2.69 MB	+2.0 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.8 ms	27.9 ms	+0.2 ms
CLI --help	65.6 ms	65.9 ms	+0.3 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/sdk-batch-runner.js	+1.6 kB	+340 B
dist/src/runtime.js	+195 B	+82 B
dist/src/session.js	-5 B	-15 B
dist/src/registry.js	+3 B	+5 B
dist/src/agent-device-client.js	-1 B	+5 B

[thymikee]

Reviewed exact head f618d020. P1: this does not satisfy #1630’s central requirement that all five callers route through one policy-driven resolution interface. selector-wait.ts never imports or consumes SELECTOR_RESOLUTION_POLICIES.wait; its production path still calls listSelectorChainMatches directly. The new parity test maps the row to a source filename and checks marker strings, so it stays green even when the declared wait policy is completely disconnected. findAct similarly consumes only requireRect while its ambiguity/ranking pipeline remains bespoke. Route wait and mutating find through the declared interface, replace source-sniffing with fixture-tree tests that drive that interface, and prove semantics remain unchanged. Substantive CI is also still pending; no readiness label.

[thymikee]

P1 addressed at 180613482. The finding was correct on all three counts, and the third one is the important one: my parity test sniffed marker strings in source files, so it stayed green across exactly the gap you found. That is asserting about the layer I edited rather than the behavior it produces — the test could not have caught a disconnected row by construction.

One policy-driven interface, genuinely consumed. resolveSelectorChainWithPolicy returns a discriminated outcome (none / resolved / ambiguous) rather than a nullable node, because the rows genuinely disagree about what several matches mean — collapsing that into node | null is what forced every caller to re-derive its contract inline in the first place.

• selector-wait.ts now resolves through it with SELECTOR_RESOLUTION_POLICIES.wait; the Design read-only identity verification without breaking wait polling #1349 landmark check still receives the candidate set (the row is first-match, so it never refuses — the adapter only reshapes, it does not re-decide).
• find.ts selector branch routes through it with findAct, and additionally asserts the row still says reject-candidates instead of assuming. The locator branch keeps its own matcher — it matches by fuzzy text scoring, not selector chains — and joins the shared contract at the narrowing step; that's now stated in a comment rather than left implicit.

Test rebuilt on fixture trees driven through the interface, no source sniffing: unique/no-match under every row, tiebreak winner and match-count disclosure for disambiguating rows, fail-closed refusal, first-match head selection, full candidate set for reject-candidates, and the rect column.

Wiring verified revert-sensitive — the proof the previous version couldn't produce:

• flipping wait to fail-closed → policy tests fail;
• flipping findAct to first-match → real find handler tests fail (ambiguous match lists snapshot-line candidates capped at 5, …lists them all uncapped, click prefers semantic controls over matching containers).

One behavior nuance the fixture work surfaced, now pinned as its own test: disambiguation declines on genuinely indistinguishable candidates (identical label, depth and area) — the tiebreak is evidence, not a coin flip — so an acting row surfaces ambiguity there instead of silently binding one. My first fixture assumed otherwise and the test caught me.

Green at this head: typecheck, lint, format, layering, fallow audit + production-exports, and 258 files / 2,223 tests across src/commands + src/daemon.

[thymikee]

Re-reviewed exact head 180613482. The previous disconnected-policy finding is addressed, but the replacement introduces a behavior regression:

• P1 — wait now discards later selector matches before landmark verification. For the first-match row, resolveSelectorChainWithPolicy returns a resolved outcome containing only matchedNodes[0]; policyMatchList reconstructs the candidate list as that single node. ADR 0012/Design read-only identity verification without breaking wait polling #1349 requires the replay landmark guard to succeed when some selector match carries the recorded identity. Previously listSelectorChainMatches passed the full set, so a screen whose first match is an impostor and second match is the recorded landmark succeeded; this head keeps polling and eventually reports wait_landmark_identity_mismatch. Preserve the full candidate set for landmark verification and add a regression with first-impostor/second-landmark. Current landmark tests use one match per capture, so they cannot catch this.

The find route looks preserved. Fallow is also red because this branch is based before merged #1642 and audits its dynamic-import helper as unused; rebase/rerun against current main is required. No readiness label.

[thymikee]

Fallow Code Quality fixed at 633c41a20 (branch rebased onto current main).

Cause, and why it isn't the policy work. The failing symbol is pinOwnProcessStartTime in src/__tests__/test-utils/host-process-mock.ts — a file from #1642, which merged after this PR opened. It reports as an unused export but is consumed by three suites; they call it as (await import('…/host-process-mock.ts')).pinOwnProcessStartTime(importOriginal) inside vi.mock factories. Vitest hoists those factories above static imports, so the dynamic form is required there, and fallow cannot trace the consumers statically.

Latent on main, not introduced by either PR. The audit gate is changed-files-only, so on main the file is never in scope (verified: a clean origin/main worktree audits 1 file and passes). It surfaces on the first PR whose diff contains it — mine, after the rebase.

Fix: a documented fallow-ignore-next-line unused-export at the declaration, with the hoisting reason recorded above it. I deliberately did not restructure the helper to be statically visible — that would break the very hoisting contract the mock depends on.

Verified at this head: fallow audit + production-exports, typecheck, lint, format, layering all green; the three affected suites 14/14; src/commands + src/daemon 258 files / 2,223 tests green. (No comment_id threads to reply to — the only PR comment was the size-report bot, whose +2.3 kB is the new resolve-with-policy module and its façade wrapper.)

[thymikee]

One additional scope blocker from #1630’s acceptance criteria: this head still does not eliminate mutating find’s two-engine chain. find.ts resolves/ranks/promotes a candidate, then handleFindClick/fill/focus/type redispatches its @ref through the ordinary interaction resolver, which re-resolves/promotes/applies guards. Changing the first matcher to the policy interface does not satisfy “Mutating find no longer chains two resolution engines.” Complete that route migration or change Closes #1630 to Part of with the remainder tracked.

[thymikee]

Re-review at 633c41a2: the two previously reported blockers remain unresolved.

• P1 — wait landmark verification still loses later candidates. resolveSelectorChainWithPolicy(..., first-match) returns only the first node, and policyMatchList reconstructs a singleton. A first same-selector impostor still prevents a later matching landmark from satisfying the poll; there is still no first-impostor/second-landmark regression.
• refactor: native target resolution — one pipeline, per-caller policy as data #1630 scope — mutating find still chains two resolution engines. find resolves a match, then redispatches its @ref through the ordinary interaction resolver; findResolvedTarget skips frame admission but does not pass the resolved node/evidence through the policy interface.

The new commit only rebases and adds the Fallow suppression. Current red checks are not evidence of these code findings: Select affected kernels and iOS Release failed downloading GitHub actions (500/503), while iOS Smoke hit an unchanged partial-typing/device flake. Reruns are needed, but the code blockers already make this not ready.

[thymikee]

Both findings addressed at c2afd78c1.

P1 — you were right, and it was a real regression I introduced, not a missing test. Routing wait through the policy interface collapsed the candidate set to the winner, and the #1349 landmark check is satisfied when some match carries the recorded identity — so a first same-selector impostor hid a later genuine landmark and timed the wait out. My "semantics unchanged" claim was wrong on exactly this path.

Fix: the resolved outcome now carries matchedNodes, the full candidate set of the alternative the winner came from, so a policy that picks one node no longer discards the rest; wait passes it straight to the landmark check. Regression test added at the shape the existing suite did not cover — both candidates in the SAME capture, impostor first (the existing test covers impostor-then-landmark across polls). Verified it goes red against the singleton reconstruction it replaces, and green with the fix.

#1630 scope — descoped honestly rather than half-done. You're right that mutating find still chains two engines: click/fill re-dispatch by @ref and re-resolve. I did not fold that into this PR because it is a dispatch-behavior change, not a declaration refactor — the invoke() hop is what currently gives find's click/fill the whole interaction leaf (action recording, ref-frame effects, settle/observation, deferred-outcome marking, and the ADR 0011 guarantee cells that ride with them). Bypassing it means deciding per behavior which of those find keeps, in a PR whose contract is "semantics unchanged".

So: retitled to Part of #1630, remainder filed as #1654 with the reasoning, the suggested shape (pass the resolved node + evidence into the leaf instead of a bare ref), and acceptance criteria including re-verifying the ADR 0011 cells and a test that fails if a second resolution happens.

Green at this head: typecheck, lint, format, layering, fallow audit + production-exports, and 258 files / 2,224 tests across src/commands + src/daemon. Agreed on the red checks being infrastructure (actions download 500/503) and the unchanged iOS partial-typing flake — those reruns are independent of these findings.

[thymikee]

Re-reviewed exact head c2afd78c. The wait-landmark P1 is fixed with the correct same-capture impostor-first regression, and the mutating-find second-resolution work is now honestly split to #1654. One matrix-truth blocker remains:

P2 — the structural policy columns are still declaration-only while the code and PR claim they are gate-tested. occlusion, offscreenGuard, promotion, and poll are never consumed by resolveSelectorChainWithPolicy or selectorResolutionKnobs; the rewritten fixture-tree suite exercises only ambiguity and requireRect. Changing any structural column leaves behavior and the gate green, contrary to resolution-policy.ts and the PR body’s “declared-plus-gate-tested” claim.

Either narrow the matrix/PR claims to the ambiguity + rect contract actually wired here and track the structural-policy remainder, or route those fields into typed production behavior and pin them behaviorally. Do not retain unverified registry claims: they can silently drift into fiction. Current CI is incomplete; the native-fingerprint failure is a GitHub action-download outage, not this finding.

[thymikee]

P2 addressed at 9d443110c — I took the "narrow the claims" branch, because the columns were fiction and I'd rather delete fiction than ship it pending.

What was wrong. occlusion, offscreenGuard, promotion, and poll were consumed by nothing. Flipping any of them changed no behavior and no test. Worse, the reason my first revision looked verified is that its parity test grepped caller files for marker strings — which is exactly why that same test also stayed green when the wait row was disconnected entirely, the defect you found last round. Same root cause both times: asserting about the shape of the code I'd just written instead of the behavior it produces.

Fix. SelectorResolutionPolicy is now { ambiguity, requireRect } — precisely the fields resolveSelectorChainWithPolicy consumes and the fixture-tree suite pins. The four structural fields are gone from the type and every row.

Guard against them creeping back. A new test asserts each row's field set is exactly ['ambiguity', 'requireRect'], so an unenforceable column cannot reappear without coverage. Verified by re-adding occlusion: true to the act row and watching it fail, then removing it.

Remainder tracked as #1656 — routing the structural stages into typed behavior — with the constraint stated up front that each field must be consumed by the interface and pinned by a fixture-tree test, not declared and grepped. PR body's table and claims updated to match what is actually enforced.

Green at this head: typecheck, lint, format, layering, fallow audit + production-exports, 258 files / 2,225 tests across src/commands + src/daemon.

[thymikee]

Re-reviewed exact head 9d44311. The previous structural-column P2 is fixed: the matrix now declares only ambiguity and rect, both consumed and behaviorally pinned. One package-boundary blocker remains. P2: the string-only selectors facade leaks the private AST through the new policy outcome. PolicyResolutionOutcome.resolution is typed as AstSelectorResolution, and the root @agent-device/selectors wrapper returns that object unchanged. Production code confirms the leak by reading outcome.resolution.selector.raw in selector-wait.ts. PR #1589 deliberately made the root facade string-in/string-out and confined parser objects to @agent-device/selectors/ast; this change reopens that boundary indirectly. The existing boundary gate only filters named exports such as Selector and SelectorChain, so it stays green on a nested return-type and runtime leak. Flatten the internal outcome at the package boundary so resolution.selector is a string using the public SelectorResolution shape, keep AstSelectorResolution package-private, and add a facade regression that fails if resolveSelectorChainWithPolicy returns an AST selector object. CI is currently pending; no readiness label.

[thymikee]

Fixed at 3d151b5bf. The leak was real and your reading of why the gate missed it was exactly right: it filters export names, and a type reached through a nested field exports no name of its own.

Fix at the layer you named. PolicyResolutionOutcome now lives in public-resolution-types.ts beside SelectorResolution, with its resolution.selector typed as text. The parser-side shape is renamed AstPolicyResolutionOutcome and stays package-private; the façade wrapper flattens on the way out — the same treatment resolveSelectorChain already gave AstSelectorResolution (#1589). selector-wait.ts reads outcome.resolution.selector now; that .raw was the runtime proof the boundary had reopened.

Two pins, both verified red against the shape they replace.

Behavioral, in the fixture-tree suite — the façade returns selector text under every policy row, on both branches that carry a selector. Reverting the flattening:

× the façade returns selector TEXT under every policy, never a parser node
  Tests  1 failed | 11 passed (12)

Structural, in R11 — resolution shapes must be re-exported from public-resolution-types.ts, not from a parser-side module. This is the fact that distinguishes a leak from a correct re-export in a name list, which is precisely what the existing assertion could not see. Restoring the pre-fix re-export site:

selectors façade must publish resolution shapes from public-resolution-types.ts, not from
the parser-side modules
+ [ 'PolicyResolutionOutcome' ]
- []

It needed a new AST helper (readReExportSources) since the existing readers return names without their source module.

One note on why I added the structural pin rather than only the behavioral one you asked for: the behavioral test proves this function is clean, but the class of defect is "any nested parser type on the façade", and the next one would arrive on a different function with the same green name list. Reading the re-export source is cheap and names the invariant directly.

pnpm check:affected --run fully green at this head, including coverage (3,375 tests) and the package/replay-compat lanes.

---

Generated by Claude Code

[thymikee]

Re-reviewed exact head 3d151b5. The prior package-boundary P2 is fixed: the public policy outcome now uses the string-based SelectorResolution shape, the façade flattens the parser-side result, and production no longer reads selector.raw. The behavioral façade regression, wait regression, package-boundary/facade gates, and full TypeScript build typecheck all pass locally. The remaining mutating-find and structural-pipeline work is honestly tracked in #1654 and #1656 under Part of #1630. No new code findings; the PR is merge-ready from review. CodeQL is queued, with no confirmed CI failure or conflict.