mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
10ff339d14
* 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>
123 lines
6.4 KiB
TypeScript
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;
|
|
}
|