9 Commits

Author	SHA1	Message	Date
Michał Pierzchała	a853734f0c	fix(webdriver): give cloud session creation its own budget and stop leaking billed sessions (#1782) ... * fix(webdriver): give cloud session creation its own budget and stop leaking billed sessions Cloud lease allocation ran under the generic 30s/1-retry request policy, so BrowserStack iOS real-device session creation (45-90s) aborted client-side at ~60s on most runs. Each timed-out POST /session still completed server-side and, being non-idempotent, was retried — leaving two billed provider sessions per failed open with no id to release them. - POST /session is its own phase: a 180s create budget (default), zero retries, and no request-bound abort, so the daemon always learns the session id. - lease_allocate carries a 300s allocation budget surfaced to providers as LeaseLifecycleContext.deadline, and a matching 330s client envelope that preserves the daemon on timeout (a reset would SIGKILL mid-create and orphan every billed session the daemon held). - The request's cancellation signal is ownership evidence: a session that completes after the requester left is released, not registered; a create that the transport gives up on surfaces typed evidence (provider + lease) so an operator can find and stop the maybe-orphaned session. Closes #1774 * refactor: one canceled-request error, and tighten the #1774 shapes Review pass over the session-create fix: - The canceled-request error had nine hand-rolled copies (src/request/cancel, maestro shared, exec, retry, install-source x2, and the new provider one). It now has one definition in @agent-device/kernel/errors: createRequestCanceledError(details?, cause?) + isRequestCanceledError + REQUEST_CANCELED_REASON. Callers add evidence or a sharper hint; the reason itself is not overridable, so nothing can build one the predicate misses. - lease_allocate's timeout bundle moves beside INSTALL_TIMEOUT_POLICY in the registry (same {...DEFAULT, envelopeMs, onTimeout} shape); the request timeout constant stays exported from timeout-policy like its siblings. - Transport: fetch helper returns Response's own ok/status; the timeout reason const is private behind isWebDriverRequestTimeout. - Client: one-use options type inlined; the two deadline helpers share one floor. - Session-manager tests: shared makeRuntime/jsonResponse/afterEach restore. Net -29 lines with the feature in. * chore: keep the canceled-request reason private to the kernel * fix: typed cancellation everywhere + own the AWS remote-access ARN through startup Second-order follow-ups the #1774 refactor made cheap: - markRequestCanceled aborts the request signal WITH the kernel's typed canceled error as its reason. Every signal.throwIfAborted(), aborted fetch, and 'throw signal.reason' in the daemon (20+ sites) now surfaces a canceled request as such instead of a bare DOMException that normalized to UNKNOWN — and no site has to know the factory exists. - AWS Device Farm prepareSession owns the remote-access ARN from the moment create-remote-access-session answers: a startup timeout, the allocation deadline, or a canceled request now stops it before the failure surfaces (previously a timed-out startup left a RUNNING billed session behind — the same leak class as the WebDriver session, one phase earlier). The startup wait is capped by LeaseLifecycleContext.deadline and wakes on cancellation. - BrowserStack's pre-session local app upload honors the request signal (an upload is not billed, so plain abort is right there). - lease_heartbeat/lease_release share lease_allocate's preserve-daemon policy: the rationale — the daemon owns billed sessions; a reset orphans them all — applies verbatim. Each AWS ownership test proven red without the guard (3/3). * refactor: dedupe billed-resource cleanup and lease-signal wiring Shrink pass — same behavior, less duplication: - releaseOnFailure(primaryError, release) in webdriver-utils replaces the two identical 'best-effort stop the billed resource, attach cleanupError to the primary AppError' helpers (WebDriver session + AWS remote-access ARN); shared errorMessage too. - The lease handler pulls the request signal from getRequestSignal(requestId) like every sibling handler, instead of threading a requestSignal arg through LeaseHandlerArgs and the request-handler chain. Drops the field, the wiring, and five mechanical test edits; the handler test now proves the request-bound signal (abort it, watch the provider's signal flip) rather than arg identity. - Inlined the one-use requestHeaders back into fetchWebDriver. Handler-signal test proven red without the wiring. * fix(lease): the daemon releases a lease allocated for a gone requester; honest release evidence Review follow-up. The provider was doing the daemon's job: it treated the request signal as 'ownership evidence, not an interrupt' and needed three paragraphs to say so. The daemon owns the request, so it now decides — generically, for every provider — what happens to a lease that finished allocating after its requester left: release it (provider + registry) and answer with the canceled error. - lease.ts: after allocate returns, isRequestCanceled(requestId) → releaseAllocationForGoneRequester(). Release evidence is claimed ONLY on a clean release (no warnings, no throw); a WEBDRIVER_SESSION_DELETE_FAILED release is reported released:false with providerSessionId + a stop-by-hand hint (thymikee's finding: the previous evidence was success-shaped even when DELETE failed). - WebDriverSessionManager: the createOwnedSession/releaseCanceledSession trio is gone; allocate is plain 'create with a budget; on failure clean up' again. - LeaseLifecycleContext.signal is just cancellation, like everywhere else; the ownership-semantics comments on the contract, client, registry, AWS prepare and utils shrink to what the code no longer says itself. - Tests: the two provider-level cancellation tests move to the daemon handler (where the logic now lives), plus the failing-DELETE regression; both proven red without the post-allocate check. * fix(aws): the allocation deadline bounds remote-access startup, not the 120s default Live iOS real-device run: startup needed ~128s and hit the standalone 120s default while the daemon's 300s allocation budget still had room — the new ownership guard correctly stopped the ARN, but the open failed for no reason. When the daemon supplies a deadline it is the bound; the default only applies standalone. Rerun: open in 112s, snapshot, clean close, session STOPPING. * test(aws): pin that the allocation deadline outlives the 120s startup default; drop empty import Review follow-ups on 7f9d1481a: a virtual-clock test (Date.now advanced 10s per poll, RUNNING at 150s, deadline 300s) that fails on the old min(default, deadline) logic and passes now; and the empty 'import {} from kernel/errors' left in maestro/shared.ts is removed. * refactor: finish the dedupe — one release path, kernel errorMessage, AWS on releaseOnFailure Code-quality review at 7f9d1481a: 1. aws-device-farm.ts still carried its own copy of releaseOnFailure (the dedupe commit's script aborted before reaching it and I mis-verified). Now uses the shared helper; private copy deleted. 2. Empty 'import {} from kernel/errors' in maestro/shared.ts removed (273870099). 3. errorMessage() lives in @agent-device/kernel/errors; the two copies this PR had added (lease.ts, webdriver-utils.ts) import it. Sweeping the pre-existing copies is a follow-up. 4. lease.ts has ONE release path: releaseLease(registry, provider, lease, request, ctx) → { released (registry), provider } used by both the lease_release case (wire shape unchanged) and the gone-requester branch, which folds a throwing provider release into releaseError. 'released' now means the same thing in both; the provider verdict is a separate 'providerReleased' (warnings-free, no throw) that drives the stop-by-hand hint. -~35 lines. 5. sessionCreateTimeoutMs is Omit-ed at the WebDriverTransportOptions boundary instead of Pick-ed back out internally. * fix(lease): 'released' on a canceled allocation means the billed session is confirmed gone Re-review at 3665ea06: unifying the release path had made the cancellation error report released:true from the daemon's registry record while the provider DELETE had failed — success-shaped again, with the operator verdict demoted to a second key. Fixed at the source of the ambiguity: - LeaseReleaseOutcome names its bookkeeping field registryReleased. - On the canceled error, 'released' is true only when registryReleased AND the provider released without warnings AND without throwing; the registry record is exposed as 'registryReleased'. The stop-by-hand hint keys on 'released'. - lease_release keeps its existing wire field ('released' = registry; provider cleanup rides in 'provider'), unchanged. - Regressions: failed DELETE and throwing release both pin released:false / registryReleased:true (+ providerSessionId, warnings|releaseError, hint); both proven red on registry-only semantics. * ci: retrigger default-setup CodeQL Run 32051017472 is wedged on GitHub's side: status=completed with Analyze (python) still queued and Analyze (java-kotlin) failed only at SARIF upload (503, 'No server is currently available'). It can be neither cancelled nor rerun, and default-setup CodeQL has no dispatchable workflow, so a new push is the only way to get a fresh run. No source change. * test(webdriver): assert the typed timeout contract on the shared-budget probe main's #1790 tightened this test to expect the raw TimeoutError DOMException, which this PR intentionally normalizes into AppError{reason: webdriver_request_timeout}. On the merge ref the two met and Coverage went red. The regression now asserts the structured contract and that the second request's budget is the shared remainder (~118ms of 200 after an 80ms first call).	2026-08-18 15:48:58 +02:00
Michał Pierzchała	60f6356b04	fix: read replay scripts on the caller and ship them with the request (#1810) ... * fix: read replay scripts on the caller and ship them with the request Closes #1802 * test: assert the caller-side replay path as a substring, not a hand-escaped regex * perf(cli): load the Maestro engine only when a replay entry is a flow The command registry evaluates every command family on CLI startup, so the replay script-source builder's static @agent-device/maestro import put the YAML parser on the --help path. It now loads on demand behind the format check, and the startup import-closure guard covers the engine the way it already covers node:http. * refactor: share the replay request field vocabulary across the CLI and client views The new replay script-source flags appear in both CliFlags and CommandExecutionOptions, which fallow flagged as a clone; ReplayRequestFields declares them once. The test-suite handler's missing-sources rejection now travels the typed-error path its sibling rejections already use, so the fix adds no branch to handleSessionReplayCommands.	2026-08-18 14:56:34 +02:00
Michał Pierzchała	0d3b7413c5	fix: prevent private AX subtree leaks at source (#1807) ... * fix: prevent private AX subtree leaks at source * fix: preserve values in settle signals * fix: normalize settle signal semantics	2026-08-18 09:23:39 +02:00
Michał Pierzchała	ed1d44fa62	fix: unify private AX scroll visibility (#1798) ... * fix: unify iOS scroll snapshot visibility * fix: preserve composed scroll hints	2026-08-18 00:47:25 +02:00
Michał Pierzchała	8c800ae53f	refactor(contracts): one viewport-root predicate for the whole repo (#1613) ... * refactor(contracts): one viewport-root predicate for the whole repo "Is this the Application/Window root" was written nine times: three spellings normalizing `type|role|subrole`, five lowercasing `type` alone, and one comparing the normalized type for EQUALITY. Two of the nine sat in `contracts/snapshot-visibility.ts` itself, disagreeing with each other. Measured before collapsing, using #1592's method — ground the comparison in what each backend ACTUALLY emits, not in fixture strings. Over the 31 names iOS's `elementTypeName` can return, the 18 fully-qualified class names Android emits, and the 24 mapped/raw forms the macOS helper produces, the nine agreed on 71 of 73. The two exceptions are macOS window subroles, and the only spelling that disagreed is maestro's `===`, whose platform union is `android | ios` — so it can never see them. The duplication was textual, not behavioral, which is what made the collapse safe. `isViewportRootNode` reads role and subrole because the macOS helper is the only backend populating them and the only one able to emit a window whose `type` does not say so: `normalizedSnapshotType` returns the raw subrole for a non-standard window, so an `AXWindow` with subrole `AXSystemDialog` or `AXUnknown` reads as neither from `type` alone. Those two shapes are the whole behavioral delta of this change, at the six call sites that were type-only, and they are windows by role. `snapshot-viewport-root.test.ts` pins the predicate over those three emitted vocabularies. Red evidence: reverting the canonical definition to the type-only spelling fails 2 of 5 cells, to the equality spelling 4 of 5. Also drops two kernel re-declarations this made visible: maestro's local `containsPoint` and `rectsOverlap` were character-identical to `@agent-device/kernel/rect`'s `containsPoint` and `isRectVisibleInViewport`, in a file that already imports from that module. And `resolveViewportRect` loses three `as Rect` casts that only existed because `.filter()` cannot narrow `node.rect` — one `flatMap` states the same thing honestly. Deliberately NOT in this change: the three viewport RESOLVERS still diverge, and on Android that is a live defect rather than duplication. Filed separately with the measurement. * test(contracts): enumerate the macOS emitter's real vocabulary Review found the table claimed to pin "the vocabulary each backend actually emits" while omitting most of it. `normalizedSnapshotType` has three output classes and only two were represented: 1. thirteen roles mapped to fixed short names — six were missing (StaticText, TextField, TextArea, MenuBarItem, Menu, MenuItem); 2. AXWindow, whose output is the SUBROLE unless it is AXStandardWindow; 3. the `default:` arm, `subrole ?? role`, emitting the raw AX-prefixed value for every unmapped role. All three are now enumerated, and the table asserts its own completeness against the emitter's fixed-output set — a role added to that switch without being added here fails, which is the emitter-drift protection the docblock was promising but not delivering. Re-measuring over the complete tables also corrected the header's own numbers. The claim was "71 of 73 agree, 2 disagree"; over 75 names it is 71 agree and FOUR disagree, because AXSystemDialog and AXUnknown were absent from the old table. Those two are the behavioral delta of this PR — an AXWindow whose subrole is emitted as the type, invisible to the six type-only spellings and named exactly by `role` — so the incomplete table had been hiding the very rows that justify reading role/subrole. The other two (AXFloatingWindow, AXSystemFloatingWindow) remain inert: only the `===` spelling misses them and its platform union is `android | ios`. * test(contracts): derive the macOS fixed-output set from the emitter Two test-validity defects from review, both real. The raw-fallback row `{ type: 'AXSearchField', role: 'AXTextField', subrole: 'AXSearchField' }` was unreachable: the `AXTextField` arm returns `TextField` whatever the subrole, so no emitter run can produce it. Replaced with `{ type: 'AXSortButton', role: 'AXCell', subrole: 'AXSortButton' }` — a subrole on a genuinely unmapped role, which is what the `subrole ?? role` default arm actually emits. `MACOS_FIXED_OUTPUTS` was a hand-kept twin compared against a hand-kept table, which is circular: a new mapped Swift role is absent from BOTH, so they agree and the gate stays green. The "emitter-drift protection" the docblock promised did not exist. The set is now parsed out of `normalizedSnapshotType` in SnapshotTraversal.swift, so the comparison is against the emitter rather than against a copy of the table's own assumptions. `case "AXWindow"` returns a subrole expression rather than a literal and is deliberately outside the literal-return set. Red evidence: adding `case "AXDisclosureTriangle": return "DisclosureTriangle"` to the Swift switch fails with `expected [ 'DisclosureTriangle' ] to deeply equal []`; 6 pass once reverted. The parser throws rather than silently matching nothing if the function is renamed or moved. * chore: restore maestro conformance corpus to main 45 corpus YAMLs carried an unrelated quote-style churn ("Button" -> 'Button'). They were already modified in the worktree when this branch started and a `git add -A` swept them into the predicate commit. Nothing in this PR reads them. Restored verbatim to main.	2026-08-05 18:16:58 +02:00
Michał Pierzchała	20e903c117	fix(maestro): unify the scrollable-ancestor walks and fix Android scroll-container selection (#1592) ... * refactor(maestro): collapse the duplicate scrollable-ancestor walk fallow reported three structurally identical "walk up the parent chain to the nearest scrollable ancestor" implementations as clone groups (dup:1b401a24, dup:ce01e1de). Two of the three predicates classify identically, one does not. snapshot-policy.ts's isScrollableNode is logically identical to contracts' isScrollableNodeLike -- same six type patterns, same `=== 'table'` equality, same role/subrole fallback, and neither normalizes the type first. The walks match too, so findScrollableAncestorRect collapses onto findNearestScrollableAncestor with `(n) => Boolean(n.rect)`. runtime-port-geometry.ts's isScrollableSnapshotType does NOT agree. It equality-matches the NORMALIZED type, so over 227 node-type strings harvested from the repo's fixtures and tests it disagrees in both directions: Android ListView/GridView/RecyclerView, HorizontalScrollView, AXScrollBar and role-only scrollables clip but are not swipe containers, while XCUIElementTypeTable and AXTable are swipe containers but do not clip (the clip predicate compares 'table' against the unnormalized type, so prefixed forms miss). Only bare `table` satisfies both. It stays separate, with the divergence and the reason each call site needs its own answer written down where it can be read. The new test is load-bearing rather than decorative: replacing isScrollableSnapshotType with the contracts predicate leaves `pnpm maestro:conformance` at 46/46 and the pre-existing maestro suite at 206/206 green. The oracle does not cover scroll-container selection, so nothing else in the repo fails on that collapse. resolveRootViewport is deliberately left alone -- it resembles contracts' resolveViewportRect but lacks its third "largest containing rect of any node" fallback, so it is a real divergence and not the next dedup. * fix(maestro): recognize Android scroll containers when aiming scrollUntilVisible The divergence note added in the previous commit was wrong, and it was covering for a bug rather than describing a design. Grounding the comparison at the call site instead of in fixture text changes the answer. `node.type` is never normalized on the way in -- it carries the raw platform string -- so the domain of each predicate is exactly what each platform emits: iOS `elementTypeName` returns 31 fixed short names ("Table", "ScrollView", "CollectionView", ...), never "XCUIElementType*". Android `attrs.className`, fully qualified. macOS role-mapped short names; outside Maestro's platform union. Over all 31 iOS names the two predicates agree on every single one. The claimed `XCUIElementTypeTable` / `AXTable` divergence was measured on strings the runner cannot produce; the real emission is "Table", which both predicates accept. The role/subrole arm is macOS-helper-only, so it is inert for Maestro entirely. What remains is Android, one-directional, and a defect: matching a normalized type for EQUALITY recognizes bare `android.widget.ScrollView` and silently misses HorizontalScrollView, NestedScrollView, RecyclerView, ListView and GridView. `scrollUntilVisible` therefore selected no container and fell back to a screen-centred swipe inside essentially every RecyclerView-backed list -- contradicting the function's own documented intent, and contradicting the existing Android test that expects `android.widget.ScrollView` to be selected. So the third walk collapses onto the shared helper too: the substring predicate is also the better fit for Android's open class-name space, where an allow-list would keep missing NestedScrollView and every custom subclass. All three walks now share `@agent-device/contracts/snapshot`, and the explanatory comment is gone because there is nothing left to explain. The test is rewritten to pin the classification over the vocabulary each platform actually emits, with the Android rows as the regression guard.	2026-08-05 07:58:15 +02:00
Michał Pierzchała	4f8dc3f31e	refactor: move selector engine into workspace package (#1589) ... * refactor: move selector engine into workspace package * refactor(selectors): trim the package façade to its real consumers Follow-up to the selector-package cutover, from a structural review of it. - Drop 15 façade symbols with no consumer anywhere in the repo: selectorUsesKey (added by the cutover, never called), isNodeVisible / isNodeEditable (the real helpers are contracts/snapshot's), normalizeText, splitIsSelectorArgs, IS_PREDICATE_REQUIRED_MESSAGE, four nested Replay types, SelectorDisambiguationDisclosure, and the four kernel type re-exports every consumer already imports from kernel directly. - Delete SelectorCapturePolicyInput.selectorExpression, which deriveSelectorCapturePolicy never read; the policy varies only by predicate, so it takes one now. Two of the four tests asserted that the unread parameter had no effect and could not fail; they go with it. - Return the Maestro export vocabulary to the maestro package. The cutover inlined MAESTRO_TEXT/STATE_SELECTOR_KEYS' values into the CLI call site, leaving both constants dead in the package that owns the concept and no gate over the two copies. MAESTRO_SELECTOR_PROJECTION is now the one statement of it. - Dedupe SelectorDiagnostics and SelectorDisambiguationDisclosure, declared character-for-character twice across the AST/string seam, and name the two shared option shapes once instead of five inline copies. The parser-side resolution types take an Ast prefix so the twins read as twins. - Delete three identity wrappers: parsePrivateSelector, selectorExpressionToMaestro, and the formatSelectorFailure forwarder — nothing passes it a chain any more, so the SelectorChain | string union and its branch go too. - Delete internal/index.ts, an AST barrel whose only consumer was one test in the same directory (renamed to engine.test.ts), and the match.ts pass-through that existed to feed it. - ReplaySelectorGrammar had three variants for two behaviors; 'wait' and 'ordinary' were the same path. It is 'is' | 'positional' now. - Drop the deleted src/sdk/selectors.ts from .fallowrc.json's entry list. Behavior unchanged. pnpm check green: 598 unit files / 5278 tests, smoke 35 passed / 3 live skipped, layering 71/71, depgraph 22/22, mutation config 45/45, fallow clean, package smoke sound. Counterfactual: pointing MAESTRO_SELECTOR_PROJECTION.textKeys at the state keys turns three replay-maestro-export cells red; restored before commit. * test(selectors): split the engine aggregation test by source concept `internal/index.test.ts` (renamed `engine.test.ts` when its barrel went away) was a 708-line aggregation over the whole engine — past the 500-line tripwire and mirroring no source module, so it also ran as one serial unit. It becomes five files that each mirror what they test, plus the parser cells folded into the existing parse test: resolve.test.ts alternative fallback, strict uniqueness, first-match existence resolve-disambiguation.test.ts ADR 0012 ranking: deepest, smallest-area, winner-vs-challenger disclosure, tie fallback resolve-viewport.test.ts the visibility half: on-screen beats off-screen, including inside an off-screen scroll container match.test.ts per-key matching semantics (text, role, focused, appname/windowtitle, decoded newline labels) arguments.test.ts where the selector ends and the command's positionals begin, both grammars parse.test.ts +6 grammar/escape cells beside the existing property tests The login-form tree shared by resolve.test.ts and match.test.ts moves to `__tests__/login-form-nodes.ts` rather than being copied into both. All 27 cells are carried over unchanged and still pass; no file now exceeds 224 lines. pnpm check green: 602 unit files / 5278 tests, layering 71/71, depgraph 22/22, mutation config 45/45, fallow clean over 127 changed files. * revert(selectors): keep agent-device/selectors public, behind one AST subpath The cutover removed the `agent-device/selectors` public subpath as part of tightening the API. It is in use, so the removal is reverted: the subpath ships the same ten symbols v0.20.5 shipped, with the same signatures. That has to coexist with the reason the package façade is string-only, so the AST leaves through one named door instead of the main one: @agent-device/selectors string-in/string-out; every in-repo consumer @agent-device/selectors/ast the published parser surface; one consumer, src/sdk/selectors.ts `packages/selectors/src/ast.ts` re-exports parseSelectorChain, tryParseSelectorChain, isSelectorToken, the AST-taking findSelectorChainMatch and resolveSelectorChain, isNodeVisible, isNodeEditable, and types SelectorChain / SelectorDiagnostics. `formatSelectorFailure` keeps its published `SelectorChain | string` first parameter as a shim here rather than widening internal/resolve.ts back to a union — the compatibility obligation sits at the boundary that owes it. This is strictly narrower than main, where the AST was reachable from anywhere in src/ via src/selectors/*. Two gates hold it there: facade-symbols.ts pins ./ast to exactly the v0.20.5 list, and package-boundaries.test.ts asserts src/sdk/selectors.ts is the only file outside the package that imports it. Restored alongside: the ./selectors export and tsdown entry/chunk group, the .fallowrc.json entry, the package-exports supported-subpath list, and both client-api.md sections. No CHANGELOG entry — nothing is removed any more. pnpm check green: 602 unit files / 5278 tests, smoke 35 passed / 3 live skipped, layering 71/71 (10 packages, 32 subpaths), depgraph 22/22, mutation config 45/45, fallow clean over 129 changed files, package smoke imported all 12 published entry points with publint and attw passing. Verified functionally against the built dist: the doc's parse -> findSelectorChainMatch example returns the same shapes as before, resolveSelectorChain still returns an AST `selector`, and formatSelectorFailure still accepts a chain. * fix(selectors): correct the two expectations that still assume the removal Review P1s on a792415a: restoring the public subpath left two gates asserting it was gone. - installed-package-metro.test.ts moved `agent-device/selectors` into the blocked-specifier list. It goes back to the subpath smoke set, running the same `isSelectorToken('||')` + `parseSelectorChain` check it ran before the removal, so the file's only remaining delta from main is a formatter reflow. - owner-files-no-leak.test.ts asserted `dist/src/sdk-selectors.js` was absent. It requires the stable named chunk again, and still rejects an auto-numbered `selectors2.js` fallback — the pair is what proves the restored tsdown chunk group is doing its job, verified against a clean build. PR body corrected: the removal is no longer described as intentional API tightening. * refactor(selectors): satisfy the widened fallow scope after rebase main's #1591 (the follow-up filed from this review) removed `packages/**` from .fallowrc.json's ignorePatterns, so the new package is audited for the first time. Everything below is a finding fallow could not previously see. Dead surface, all confirmed consumer-free: - 12 type re-exports from the `.` façade whose shapes consumers only ever reach structurally. - MAESTRO_TEXT_SELECTOR_KEYS / MAESTRO_STATE_SELECTOR_KEYS, orphaned by this branch's own MAESTRO_SELECTOR_PROJECTION change, and the test-util SELECTOR_VALUE_HAZARDS. All three are module-local now. - IS_PREDICATE_USAGE_HINT fails --production because its only consumer is the is-argument-surface parity test. It gets a commented `ignoreExports` entry rather than deletion: the constant is what makes the daemon and CLI raise ONE hint instead of two copied strings (ADR 0010), so the test asserting that is the point, not an accident. `fast-check` is now declared by the package that imports it. Duplication, split by what could be proven: - `isUsefulVisibilityAnchor` existed character-for-character in both packages/selectors and packages/maestro. Moved to @agent-device/contracts/snapshot, which both already depend on and which already owns this vocabulary. Safe because the `normalizeType` each copy called is itself character-identical to the contracts one — checked before moving, since a different normalizer would have silently changed which nodes anchor. - maestro additionally reimplemented `normalizeType`, `buildSnapshotNodeMap` (as `buildSnapshotNodeByIndex`) and `findSnapshotAncestor`, all character-identical to contracts'. Deleted in favour of the shared ones. - The three scroll-ancestor walks are NOT deduped. They are structurally the same walk but each uses a different scrollable predicate, and I have no evidence the three agree; collapsing them would be a Maestro-conformance change, not a cleanup. Both maestro sites now say so, and the work is filed separately. `projectSelectorExpression` (15 cyclomatic / 22 cognitive, written by the cutover) splits into a dispatcher plus `readAgreedTextValue` and `projectSelectorTerms`; all three are under threshold. Rebase note: the one conflict, in package-boundaries.test.ts, resolved to NEITHER side — #1591 had already deleted `AdReplayVerifiedTargetGuard` as an unused export, and this branch deletes the seven ReplaySelectorPort names, so the conflicting block is empty. * build: record fast-check for packages/selectors in the lockfile Declaring the dependency in packages/selectors/package.json without regenerating pnpm-lock.yaml made every CI job fail in its install step with ERR_PNPM_OUTDATED_LOCKFILE. My local `pnpm install --frozen-lockfile` printed "+ 1 dependencies were added: fast-check@^4.9.0" and exited 0, which read as success but was the same mismatch CI refuses. Regenerated with the pinned pnpm 11.17.0, not the 11.5.3 on this machine: 11.5.3 rewrites peer-dependency resolution keys repo-wide (dropping `(supports-color@7.2.0)` suffixes) and produced a 222-line diff. With the pinned version the diff is the 4 lines this change actually needs, plus pnpm's alphabetical re-sort of the root selectors entry.	2026-08-04 19:06:39 +02:00
Michał Pierzchała	eb3fc5b28d	chore: scan packages/** with fallow instead of ignoring it (#1591) ... `ignorePatterns: ["packages/**"]` landed in #1494 W0 with the recorded reason "its resolver cannot follow workspace specifiers". That was either wrong at the time or never re-checked: the fallow version has not moved (^2.95.0 then and now) and it resolves @agent-device/* through each package's exports map today. packages/kernel alone exposes 8 subpaths and ~110 exports reachable only via workspace specifiers, and scanning it reports zero findings — a resolver that could not follow the specifier would report all of them. The cost of the ignore is that every package extraction silently removes its code from dead-code analysis. #1589 moved the selector engine into packages/selectors/ and shipped a façade with 15 zero-consumer exports, including `selectorUsesKey`, written in that PR and never called. A follow-up commit removed them by hand; nothing would have caught them. Removing the pattern surfaced 43 findings, driven to zero by deleting the dead code rather than by baselining or excluding it (fallow-baselines/*.json are empty on purpose — the posture is fix-or-document-the-exemption, so a first baseline entry would be a policy change): - 38 are deleted. 24 façade type re-exports whose only claim was that a consumer might one day want to name them — typecheck is green without every one, so the claim was theoretical; 5 façade value re-exports; 9 `export` keywords on symbols used only inside their own file. Every deleted façade symbol comes off scripts/layering/facade-symbols.ts (and ad-replay's inline pin in package-boundaries.test.ts) in the same change, so R11 is narrowed with the façade, never weakened around it. - 4 stale suppressions in src/provider-limrun-runtime.ts existed only because packages/ was invisible. - 5 have consumers analysis genuinely cannot see, and get an `ignoreExports` entry naming the consumer per the existing `comment` convention: four test-tree importers that --production does not walk, and `LimrunIosCommandExecution`, which src/sdk/limrun.ts republishes as agent-device/limrun — its only importer compiles in a temp checkout, so no static edge reaches it. test/integration/limrun-public-types.test.ts is the standing proof that one is real API. Three doc comments named types their façade no longer exports and are corrected rather than left asserting something false — including #1555's claim in session-replay-target-verification.ts that the daemon imports `AdReplayVerifiedTargetGuard` directly. It does not; it reaches that shape through `AdReplayTargetClassification`/`AdReplayDispatchGuard`, which is why the name read as dead. `scripts/maestro-conformance/**` was ignored wholesale to cover its corpus data. Narrowed to `corpus/**`, which un-hides the tooling beside it and turned up one more file-local export (`buildManifest`); regenerate.mjs's importer of `fixtureContentHash` becomes visible, so that needs no exemption at all. scripts/check-affected/model.ts deliberately did not select the `fallow` check for packages/*/src/**, carrying the same stale rationale as a comment. Without that selection the new scope would never run in the affected-driven lane, so the ignore removal would have bought nothing. model.test.ts now pins the selection. Verified: check:fallow and check:production-exports green with packages in scope; full-repo `fallow dead-code` back to its one pre-existing finding; typecheck, layering (R11), lint, format, build, check:package, and the limrun published-types integration test all pass. Probed by adding a fresh zero-consumer export to the xml façade — check:production-exports reports it, so the #1589 case now fails the gate. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-04 18:26:03 +02:00
Michał Pierzchała	0e51007b04	refactor: isolate maestro engine package (#1506) ... * refactor: isolate maestro engine package * perf: deepen maestro facade boundaries	2026-07-30 20:58:20 +02:00