Files
Michał Pierzchała 10ff339d14 refactor: declare selector resolution policy as data (#1649)
* refactor: declare selector resolution policy as data (#1630)

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. SELECTOR_RESOLUTION_POLICIES
(packages/selectors) now declares one row per caller — ambiguity kind plus
the structural columns (rect, occlusion, off-screen guard, promotion, poll)
— and selectorResolutionKnobs turns a row into the engine knobs it stands
for. Callers consume rows; zero ambiguity literals remain in src.

Semantics are unchanged by construction: each row was read off its call
site. The matrix names what was previously implicit — act and get text
disambiguate, is/get attrs fail closed, exists/find-reads and wait take the
first match, mutating find rejects candidates unless narrowed (#1625).
`reject-candidates` is declaration-only and rejected by
selectorResolutionKnobs at the type level, because find enforces it through
its own narrowing rather than engine knobs.

resolution-policy-parity.test.ts gate-tests the matrix against the callers
(ADR 0011's declared-plus-gate-tested pattern): knobs must match the named
ambiguity contract, every claimed structural column must appear in the
caller's source, the read/wait pipelines must genuinely lack the machinery
they disclaim, and no caller may reintroduce an inline literal. Verified
revert-sensitive: flipping readUnique to disambiguate and faking wait's
occlusion column each fail it.

Out of scope, unchanged, per the issue: the Maestro engine (ADR 0015) and
the open click-implicit-wait product decision.

* refactor: route wait and mutating find through the policy interface (#1649 review)

P1 was right: the first head declared seven rows but genuinely routed five.
selector-wait.ts never imported its row (it called listSelectorChainMatches
directly), findAct consumed only requireRect while its ambiguity contract
stayed bespoke, and the parity test sniffed marker strings in source files —
so it stayed green across exactly that gap. Asserting about the layer I had
edited instead of the behavior it produces.

resolveSelectorChainWithPolicy is now the one policy-driven entry: it
returns a discriminated outcome (none / resolved / ambiguous) because the
rows genuinely disagree about what several matches mean, which is what
previously forced each caller to re-derive its contract inline. wait and
find's selector branch both route through it; find additionally asserts its
row still says reject-candidates rather than assuming.

The parity test is rebuilt on fixture trees driven through that interface —
no source sniffing. Wiring verified revert-sensitive: flipping the wait row
fails the policy tests, and flipping findAct fails REAL find handler tests
(ambiguous-candidate listing), which is the proof the previous version
could not produce.

One behavior nuance the fixture work surfaced and now pins: disambiguation
declines on genuinely indistinguishable candidates (the tiebreak is
evidence, not a coin flip), so an acting row surfaces ambiguity there rather
than binding one silently.

* fix(test): let fallow see the host-process mock helper's real consumers

Rebase onto main brought #1642's host-process-mock.ts into this PR's
fallow scope, where its export reports as unused. It is not: three suites
consume it, but only through `(await import(...)).pinOwnProcessStartTime`
inside vi.mock factories — vitest hoists those above static imports, so the
dynamic form is required and fallow cannot trace it statically. Documented
suppression rather than a restructure that would break the hoisting
contract.

Latent on main rather than introduced here: the audit gate is
changed-files-only, so main sees the file in scope only from a PR whose
diff contains it.

* fix: keep every candidate when a policy resolves one winner (#1649 review P1)

A real regression I introduced, not a test gap: 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.
A first same-selector impostor therefore hid a later genuine landmark and
timed the wait out.

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 throws the rest away. wait passes that straight to the landmark
check, restoring the original semantics.

Regression test added at the within-one-poll shape the existing suite did
not cover (both candidates in the SAME capture, impostor first); verified
it goes red against the singleton reconstruction it replaces.

* refactor: declare only the policy fields the matrix enforces (#1649 review)

The occlusion / offscreenGuard / promotion / poll columns were never
consumed by resolveSelectorChainWithPolicy or selectorResolutionKnobs:
changing any of them left behavior and the suite green, so they were
unverifiable claims that read as truth. (My earlier source-sniffing test
"verified" them by grepping caller files for marker strings — which is why
it also stayed green when a row was disconnected entirely.)

The matrix now declares exactly what it enforces: the ambiguity contract and
the rect requirement, both consumed by the resolution interface and pinned
behaviorally. A new test asserts every row's field set, so an unenforceable
column cannot reappear without coverage — verified by re-adding one and
watching it fail. Routing the structural stages into typed behavior is
tracked in #1656 with the constraint that each field must be consumed, not
merely declared.

* fix(selectors): flatten the policy outcome at the package boundary

`PolicyResolutionOutcome.resolution` was typed as `AstSelectorResolution` and
the root façade returned it unchanged, so the parser AST #1589 confined to
`@agent-device/selectors/ast` came back through a nested field.
`selector-wait.ts` reading `outcome.resolution.selector.raw` was the runtime
proof. The existing boundary gate reads exported *names*, so it could not see
this.

The public outcome now lives beside `SelectorResolution` in
public-resolution-types.ts with its selector as text; the parser-side shape is
renamed `AstPolicyResolutionOutcome` and stays package-private, and the façade
wrapper flattens on the way out — the same treatment `resolveSelectorChain`
already gave `AstSelectorResolution`.

Two new pins, both verified red against the shape they replace: a behavioral
one asserting the façade returns selector text under every policy row, and a
structural one asserting resolution shapes are re-exported from
public-resolution-types.ts rather than from a parser-side module — which is
what distinguishes the leak from a correct re-export in a name list.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Rva4YGtSCAKJqH5PbpcCU

---------

Co-authored-by: Claude <noreply@anthropic.com>
2026-08-06 21:10:30 +02:00

123 lines
6.4 KiB
TypeScript

// Named-export enumeration for a façade module's own source text — the R11
// structural gate in `package-boundaries.test.ts` uses this to prove a
// façade names its exports explicitly instead of a bare `export *`.
//
// Split out of `package-boundaries.ts` (#1574 review): the boundary rules
// answer "may this file import that one?", while this module answers "what
// does this façade name?" — different questions, so per AGENTS.md they are
// different bounded reads.
//
// This module used to also carry `readFacadeExports`, a ~200-line star-chain
// resolver (`GetExportedNames`/`ResolveExport` re-implemented over relative
// specifiers) that existed only to enumerate what a barrel's `export *`
// hides, feeding a hand-maintained 816-symbol pin table
// (`facade-symbols.ts`, #1574). Every workspace-package façade now names its
// exports explicitly (`export { a, b } from './x.ts'`), so there is nothing
// left for a star-chain walker to resolve — the façade file itself is the
// pin, and `readNamedExports` below is enough to prove it stays that way.
import { parseSync } from 'oxc-parser';
/**
* Every name a façade module exports, value or type-only, sorted — the exact
* "named-export-list" a package-boundaries gate can pin (#1555 review P1,
* "add the reviewer-required exact exported-symbol gate"). Covers both
* re-export forms (`export { a, b } from './x.ts'`,
* `export type { a, b } from './x.ts'`, with or without `as` aliasing — the
* alias is reported, since that is the name a consumer actually imports),
* `export * as ns from './x.ts'` (one real name, `ns`), and direct
* declarations (`export function`/`const`/`class`/`type`/`interface`,
* including `export const a = 1, b = 2`'s multiple declarators). A stray
* export — intentional or not — changes this list, so a test that pins it
* exactly turns "the façade grew a symbol" into a loud failure instead of a
* silent widening only a PR diff review would catch.
*
* AST-based (`oxc-parser`, already a devDependency — `session-state.ts` is
* the existing precedent for using it in this gate), not a regex, for the
* SAME reason `session-state.ts` gives: a regex has to enumerate every
* export FORM by hand, and the one it forgets is exactly the one that slips
* through. That is precisely what happened here (#1555 review, second pass,
* "the gate also ignores export-star declarations, so it can miss future
* widening"): `export * from './x.ts'` re-exports an unbounded, statically
* unknowable set of names — the old regex scanner had no case for it at all,
* so it silently contributed NOTHING to the list instead of failing loudly.
* `parsed.module.staticExports` is oxc's own resolved export-entry table
* (built for exactly this purpose, not re-derived from a manual AST walk),
* and its `exportName.kind` already draws the line this function needs:
* `'None'` is bare `export *` (unenumerable — thrown), `'Default'` is
* `export default …` (also thrown — a facade pinned to an exact named-export
* list must not carry one), and `'Name'` is every enumerable form above,
* `export * as ns` included (oxc reports its one real bound name, `ns`).
*/
export function readNamedExports(source: string): string[] {
const parsed = parseSync('package-boundaries-export-scan.ts', source);
const names = new Set<string>();
for (const staticExport of parsed.module.staticExports) {
for (const entry of staticExport.entries) {
if (entry.exportName.kind === 'None') {
throw new Error(
"readNamedExports cannot enumerate 'export * from …' — it re-exports an unknown set " +
'of names, exactly the widening an exact-export-list gate exists to catch. Name the ' +
're-exported symbols explicitly instead of re-exporting the whole module.',
);
}
if (entry.exportName.kind === 'Default') {
throw new Error(
"readNamedExports cannot enumerate 'export default …' as a named symbol — a facade a " +
'caller pins to an exact named-export list must not carry a default export.',
);
}
if (entry.exportName.name) names.add(entry.exportName.name);
}
}
return [...names].sort();
}
/**
* Every name a module declares or re-exports BY NAME, ignoring any bare
* `export *` it also carries. `readNamedExports` refuses such a module
* outright, which is right for a façade — a star there is unbounded widening —
* but wrong for a SOURCE the exhaustiveness gate is reading: skipping the whole
* file also skips its direct exports, so removing one of those from a façade
* would narrow the surface undetected (#1614 review P2). The starred names are
* not lost: a façade must re-export the starred module directly too, and that
* path is checked on its own.
*/
export function readDirectNamedExports(source: string): string[] {
const parsed = parseSync('facade-source-export-scan.ts', source);
const names = new Set<string>();
for (const staticExport of parsed.module.staticExports) {
for (const entry of staticExport.entries) {
// `None` is the bare star (unenumerable here, covered elsewhere);
// `Default` is never reachable through a star re-export.
if (entry.exportName.kind !== 'Name') continue;
if (entry.exportName.name) names.add(entry.exportName.name);
}
}
return [...names].sort();
}
/**
* Which module each name in `source` is re-exported FROM, for names that come
* from a re-export rather than a local declaration.
*
* A façade's export *names* are only half its boundary: a type re-exported
* from the right module and one re-exported from a package-private module read
* identically in the name list, while only the second leaks. #1649 shipped
* exactly that — a policy outcome re-exported from the parser-side module, so
* its nested `resolution` field handed callers the private AST — and the
* name-list gate stayed green throughout.
*/
export function readReExportSources(source: string): Map<string, string> {
const parsed = parseSync('facade-reexport-source-scan.ts', source);
const sources = new Map<string, string>();
for (const staticExport of parsed.module.staticExports) {
for (const entry of staticExport.entries) {
if (entry.exportName.kind !== 'Name' || !entry.exportName.name) continue;
if (!entry.moduleRequest) continue;
sources.set(entry.exportName.name, entry.moduleRequest.value);
}
}
return sources;
}