mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
perf/1961-cli-compile-cache
21 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michał Pierzchała
|
03c3984066 |
perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump (#1969)
* perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump
`@agent-device/contracts/platform` unions 32 vocabulary modules and
`/interaction` another 18. A file that value-imports either evaluates the whole
union to reach one function, and because permanent hubs sat behind them —
`command-descriptor/registry.ts`, `core/capabilities.ts`,
`interactors/register-builtins.ts`, `command-descriptor/platform-execution-entry.ts` —
that union rode into roughly half the unit suite's test graphs.
Give every vocabulary module its own entry subpath and move all value-importers
onto the module that owns the symbol. Type-only importers are left alone: `import
type` is erased, so it already evaluated nothing.
Measured with the #1950 eager-import-closure walker over all 974 unit-core test
files, against base
|
||
|
Michał Pierzchała
|
f065e6aeb4 |
fix(maestro): align label metadata ownership (#1909)
* fix(maestro): align label metadata ownership * fix(maestro): centralize command label parsing * test(maestro): cover runFlow label ownership |
||
|
Michał Pierzchała
|
ef6ec2995b |
chore(layering): document R12/R18/R19, retire R8, make R9 shrink mandatory (#1781 A6) (#1825)
* chore(layering): document R12/R18/R19, retire R8, make R9 shrink mandatory (#1781 A6) The A6 review kept `check:layering` in full (15/15 planted violations fired, no other enforcer exists) and left four follow-throughs. R12 bin-alias-fast-path, R18 contracts-implementation-authority and R19 selector-pipeline-ownership were live rules with no ADR or CONTEXT anchor — they now carry one each, in the same list as R7/R9/R10/R13. R8 zero-dep-job-closure is retired: no CI job sets `install-deps: false` and ci.yml records why each keeps it enabled, so the invariant has no subjects. R11's relative-into-packages exception existed only because a zero-dep closure cannot coexist with specifier loads, so it retires with R8; the route is now closed to every caller. R1 was retired the same way at #1490. R9 was growth-only and merely suggested lowering the ceiling, which is headroom the next change spends without a number moving. It is now an equality pin like R6 and the R10 R7 counts, and the committed baseline drops 47 -> 46 (daemon-server ceiling 17 -> 16) to match the measurement. ADR 0019 §6 now says each runtime-command-cutover row is deleted when that command's migration is declared closed. * chore(layering): rename R9 to type-cycle-size now that it fails both ways (#1781 A6) |
||
|
Michał Pierzchała
|
74eab2a554 |
refactor: route selector-resolution structural stages into typed policy (#1744)
* refactor: route selector structural stages into typed policy #1649 landed the per-caller ambiguity matrix and deliberately left four structural columns out: occlusion, off-screen, hittable-ancestor promotion, and the poll budget were per-caller pipeline code, so declaring them would have been an unverifiable claim (nothing consumed them; flipping one left the suite green). This adds the missing half as a table with runners. `SELECTOR_PIPELINE_POLICIES` (src/core/selector-pipeline-policy.ts) gives each caller ONE row naming its ambiguity contract plus its four stages, and every stage is reached only through a runner that reads the row: - occlusion -> selectorPipelineCandidates (candidacy) and resolveSelectorPipelineTarget (refusal). Acting rows exclude covered nodes and refuse covered targets; `find` and the diagnosis probe keep them as candidates and refuse at the target; reads and `wait` ignore them. - promotion -> resolveSelectorPipelineTarget. The per-call-site `promoteToHittableAncestor: boolean` is gone: click/press/longpress name `promotedTarget`, fill/focus/scroll/drag endpoints and the native-ref preflight name `resolvedTarget`. `find`'s below-the-root variant is a declared value rather than a second local helper. - off-screen -> throwIfOffscreenInteractionTarget, which now takes the row and returns the node untouched (no iOS rescue round trip) for observation rows. - poll -> selectorPollBudget, which createWaitPolling derives its deadline and inter-poll delay from; the two wait loops carry a budget, every other row carries none and cannot be polled. Behavior is byte-identical. The acting refusal keeps its exact node, label and details in every branch (promotion declines to retarget away from a covered node, so the "both covered" case names the same node it always did), and `find` carries the occlusion verdict to the focus/type seam rather than raising it early, because find click/fill still delegate that refusal to the interaction leaf's own error shape. selector-pipeline-policy.test.ts drives EVERY row through EVERY runner, including the rows whose answer is "skip" — the half that used to be an absence of code, and an absence cannot fail. Each stage was proven red by flipping its cell (occlusion, promotion, off-screen, poll, plus the declare-only-what-is-enforced guard). The ADR 0011 occlusion/nonHittable `via` pointers for the runtime tree paths now name the runner that makes the decision, not the predicate it applies. Closes #1656; prework for #1739 (waves 4-5). * docs: state constraints instead of narrating the refactor Comment pass over #1656: drop the "used to be per-caller code" / "not module constants" / "rather than an omission" narration — a comment should say what a future edit must respect, not what the previous shape was — and compress the find occlusion-verdict and poll-budget notes to the constraint they actually carry. * refactor: make the selector pipeline the only door to the engine Review of #1744: the structural rows were declared but bypassable. Read and wait routes composed `selectorPipelineCandidates(row, nodes)` with the raw `resolveSelectorChainWithPolicy(..., row.resolution)` and never entered the promotion or off-screen stages, so flipping a read row's `promotion` or `offscreen` changed only the policy unit tests — production `get`/`is`/`wait` were unaffected, which is the unverifiable-column failure #1656 exists to remove. Callers could also pair one row's candidate set with another row's ambiguity contract, and `find list` reached the engine directly. The owning interface (src/core/selector-pipeline.ts) now runs every stage a row declares, skips included, and the stage functions are private to it: - `resolveSelectorPipeline` — single-target rows: candidacy, ambiguity, the replay-guard hook, promotion, occlusion, off-screen. - `listSelectorPipelineMatches` — `reject-candidates` rows, returning the candidate set AND the tree the row sees, so ranking and equivalence classification judge the same nodes candidacy produced. - `runNodePipelineStages` — the node stages for a target from a non-chain matcher (`@ref`, find's fuzzy locator) or a narrowed candidate set. A row whose off-screen stage refuses must supply a refusal shape, so flipping an observation row to `refuse` fails on its real route instead of silently observing. `find list` now names a `readList` row (the new `reject-candidates`/no-rect ambiguity row) instead of calling the engine. R17 selector-pipeline-ownership (scripts/layering/) makes the bypass structurally inexpressible: only the owner may import the engine entry points. Proven against a planted import in selector-read.ts, which the repo-wide scan rejects with the entry points that replace it. Flips now fail through REAL command routes, verified one at a time: readUnique.occlusion/offscreen/promotion and wait.occlusion via get attrs / is / wait; readAny.offscreen via is exists and find; readList.occlusion via find list; promotedTarget.promotion via runtime click. The wait route test needed an advancing clock first — with the frozen one a refused wait spun instead of failing, so the flip hung rather than asserting. * refactor: drop find's dead candidate binding The selector branch bound the row's candidate set and never read it: only the acting classification needs that tree, and find's locator branch brings its own matcher. Names what actually governs the locator target — the shared node stages below, not a candidate set it never had. * refactor: reserve the selector engine behind the pipeline owner Review of #1744 (three blockers). **Listing rows no longer claim stages they cannot run.** `find <q> list` resolves to a candidate SET, so promotion, the off-screen guard and a poll budget have nothing to apply to — a listing has no single element to retarget, keep on screen, or wait for. `readList` now declares only the two stages a listing executes (`SelectorListPolicy`: resolution + occlusion), and the narrower shape is load-bearing: `runNodePipelineStages` and `selectorPollBudget` take the full row, so handing them a listing row is a compile error rather than a silently skipped stage. Pinned with `@ts-expect-error` — widening `readList` makes the directives unused and fails the typecheck. **The engine door is a specifier, not a symbol.** R17's regex could not see a namespace import, a re-export, or a deferred `import()`, none of which mention the symbol it matched. The two engine entries moved to `@agent-device/selectors/engine`, and R19 enforces over the resolved import graph, where every one of those forms is the same edge. Proven on the repo-wide scan by planting each form into a shipped route: namespace import, dynamic import, and `export *` laundering all come back red. `resolveImportEdges` drops an edge whose specifier resolves to nothing, so a specifier rule goes quiet — not red — if the subpath is ever retired. The gate now says that out loud instead of scanning clean. **R19, not R17.** #1750 allocates R17/R18. Verified free against origin/main and that PR's diff, then validated by real merges in both directions: the uniqueness gate passes either way and the three ids stay distinct. The gate itself is new (`scripts/layering/rule-ids.ts`): two branches taking one free number do not conflict in git, so nothing caught R17 twice. Matching whole string literals is what separates a declaration from prose that names a rule, and it is what let the gate see #1750's `const RULE = '…'` shape — the first version missed it and would have been vacuous. `main`'s two pre-existing collisions (R11, R13) are listed as known, not pinned by equality, so #1750 lands in either order without breaking this. Also: the root façade now exposes no resolver at all, and its surface test pins both doors. * fix(layering): make each rule-id allowance expire with its collision Review of #1744: `KNOWN_RULE_ID_COLLISIONS` filtered the exact R11/R13 collision strings, so once #1750 renames those rules apart the entries would keep waving those very collisions through if anyone reintroduced them. "Inert" was wrong — a stale allowance fails open, permanently. `ruleIdCollisionFailures` now checks the transition from both sides: a collision nobody allowed fails, AND an allowance whose collision is absent from the scan fails as a stale allowance. The entry therefore has to be deleted in the same change that removes the collision, and the list burns down to empty, which admits nothing. #1750 is still open, so the transitional entries stay for now (option (b)). Verified against a scratch tree carrying that PR's rename: leaving the list untouched reports both entries as stale; deleting them is clean; and reintroducing `R11 names contracts-implementation-authority and package-boundaries` afterwards is rejected. The last of those is also a unit regression, so the post-transition guarantee is pinned rather than argued. |
||
|
Michał Pierzchała
|
b15c502318 |
refactor: extract platform network runtime (#1702)
* refactor: extract platform network runtime * fix: preserve platform network recovery routes * test: guard network parser placement |
||
|
Michał Pierzchała
|
b1ed5353d1 |
refactor: extract platform log runtime (#1701)
* refactor: extract platform log runtime * fix: clear terminal app log recovery markers * fix: preserve scoped app log tooling * fix: preserve app log cancellation * fix: handle large changed coverage diffs * fix: harden Limrun runtime identity * refactor: tighten platform log runtime * fix: close app log trust gaps * fix: accept canonical session path aliases * refactor: extract durable capture kit * fix: refresh retained log marker admission * fix: rotate app logs after process relaunch |
||
|
Michał Pierzchała
|
c06bed9f77 |
refactor: extract platform device inventory runtime (#1699)
* refactor: extract platform inventory runtime * fix: preserve scoped Apple inventory tooling * fix: preserve Apple tool cancellation * refactor: tighten platform inventory boundaries |
||
|
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> |
||
|
Michał Pierzchała
|
d8b309c6db |
refactor(contracts): name façade exports explicitly and retire the pin table (#1614)
* refactor(contracts): name façade exports explicitly and retire the pin table Thirteen of the fourteen `@agent-device/contracts` façades were bare `export *` barrels. `facades/snapshot.ts`, added by #1582, was the one exception — explicit named re-exports — and that is now the rule. Everything #1574 built to cope with `export *` goes with them: scripts/layering/facade-symbols.ts -980 (816 pinned names) scripts/layering/facade-exports.ts -192 (readFacadeExports) scripts/layering/facade-exports.test.ts -234 (star semantics) scripts/layering/package-boundaries.test.ts -55 `readFacadeExports` re-implemented ESM `GetExportedNames`/`ResolveExport` — star-chain resolution, ambiguity rejection, diamond binding identity, cycle guards, spec-accurate `default` filtering at the star rather than the source. All of it existed to enumerate what `export *` hides. 523 of the 816 pinned names belonged to contracts, i.e. to those thirteen files. Once a façade names its exports, the façade file IS the pin, and it is visible in the diff of the file that widened rather than in a separate table a reviewer has to cross-check. `readNamedExports` (20 lines) stays and is enough: it already throws on bare `export *` and on `export default`. The pin is replaced by one structural gate — no façade may contain a bare star — which reuses that rejection rather than adding a regex. Surface equivalence verified independently, not asserted: main's own `readFacadeExports` run over the new façades, compared against main's own `FACADE_SYMBOLS` table — 31 subpaths, 0 added, 0 removed. Red evidence for the new gate: planting `export * from '../request-progress.ts'` back into facades/progress.ts fails it with the file named and the reason quoted; 12 pass / 0 fail once reverted. Not included: the `lowerAndroidTouchPlan` tuple-assertion drive-by. It needs `sampleGestureOffsets` to carry a min-arity tuple through `.map()`, which TypeScript will not infer without a typed helper — a real change to the gesture-plan contract rather than a drive-by, so it stays out. * test(layering): assert façades stay exhaustive over their sources Review on #1614 caught this conversion silently narrowing the public surface. The explicit lists were generated against the surface at fork time; #1567 landed 13 exports meanwhile — `DragOptions`, the drag-gesture vocabulary (`COORDINATE_GESTURE_KINDS`, `CoordinateGesturePayload`, the three `DEFAULT_DRAG_*` constants, `DragGestureInput`, `DragGesturePayload`, `GestureCommandInput`, `buildDragGesturePlan`, `dragGesturePayloadFromPositionals`, `normalizeGestureCommandInput`) and `MultiTargetAnnotationV1`. The `export *` barrels had been forwarding all 13 automatically; the rebase dropped every one, and only a human diff caught it. The star-rejection gate could not: it only proves a façade does not WIDEN invisibly. Narrowing is the failure an explicit list newly makes possible, because `export *` could not narrow by construction. So the property the stars gave for free is now asserted directly — every name a re-exported source declares must appear in the façade. Scoped to `packages/*/src/facades/`, the barrels this PR converted. A hand-curated package `index.ts` is a different thing: `ad-replay` deliberately publishes two values out of a much larger `internal/`, and forcing exhaustiveness there would widen a surface its owner narrowed on purpose (#1555). A source that itself carries a bare `export *` is skipped — unknowable from that file alone, and reachable because the façade re-exports the starred module directly too, which IS checked. Red evidence: dropping `MultiTargetAnnotationV1` from facades/replay.ts — one of the 13 the old gate was blind to — fails with the file, the source and the symbol named. 13 pass / 0 fail once restored. * fix(layering): close the exhaustiveness gate's starred-source hole Two review findings, plus a third the gate caught on itself. P1 — the three `DEFAULT_DRAG_*` constants join the existing public-façade suppression, alongside `COORDINATE_GESTURE_KINDS` and `normalizePublicGesture` which the same conversion surfaced. All five are #1567's drag vocabulary, made individually visible to `--production` analysis for the first time because a bare star used to hide them from that exact check. Kept rather than narrowed, for the reason the existing entry already states: the façade's surface stays byte-identical to what the retired pin table asserted, and narrowing is a follow-up with its own review. P2 — the exhaustiveness gate skipped any source carrying a bare `export *`, which dropped that module's DIRECT exports from the check too. `gesture-plan.ts` stars `gesture-plan-types.ts`, so removing `buildDragGesturePlan` from the façade narrowed the public surface and still passed. `readDirectNamedExports` now reads exactly the names a module declares or re-exports BY NAME and ignores the star, so direct exports are checked while the starred set stays covered by the façade's own direct re-export of that module. Red evidence: removing `buildDragGesturePlan` from facades/interaction.ts now fails naming file, source and symbol; 13 pass / 0 fail restored. Third, and the reason the gate is worth having: rebasing onto main after #1612 merged silently dropped `TEXT_ENTRY_ROUTES`, `TextEntryRoute` and `TypeTextBackendResult` from the interaction façade — the same narrowing class as the #1567 one review caught by hand, one merge later. The gate failed on it before CI did. Restored. |
||
|
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.
|
||
|
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> |
||
|
Michał Pierzchała
|
bcaa106845 |
refactor: extract snapshot and replay identity semantics (#1582)
* refactor: extract snapshot and replay identity semantics * refactor: move pure rect primitives from contracts to kernel/rect containsPoint, pickLargestRect, and isRectVisibleInViewport are raw rectangle arithmetic with no snapshot awareness, so they belong beside rectContains/rectArea in @agent-device/kernel/rect rather than in the snapshot-semantics vocabulary. The node-aware resolveViewportRect folds into contracts/snapshot-visibility.ts, retiring snapshot-geometry.ts; after this split, everything behavioral in @agent-device/contracts/snapshot is policy that interprets the snapshot model. * refactor: restore ADR-0012 rationale docs and dedupe replay identity shapes The #1478/#1581 extraction moved the identity/structural helpers but compressed their invariant documentation to one-liners; the deliberate no-ancestry-exclusion rule on idMatchCountInTree, the fail-closed guard comparison, and the who-throws/who-detects contracts on the two divergence reason markers now travel with their definitions again. LocalIdentity and NodeStructuralDenotation move to contracts/target-annotation.ts (beside TargetAncestryEntry, which WaitLandmarkMismatchEvidence now references directly), so the guard shapes in contracts/replay.ts are nominal instead of hand-rolled structural twins; ad-script re-exports the vocabulary beside the readers that produce it. Also inlines the demoteNonUniqueId pass-through wrapper in session-target-evidence.ts. * style: fix oxfmt formatting in snapshot-visibility * refactor: move findSnapshotAncestor into contracts/snapshot-tree The last root value import from src/selectors: predicates.ts reached src/snapshot/snapshot-processing.ts for the ancestor walker. The walker is index-based tree traversal with no presentation policy, so it joins buildSnapshotNodeMap in contracts/snapshot-tree.ts; both consumers repoint to the façade and the non-contiguous-index/cycle coverage moves to the package test. src/selectors now has zero value imports from root src in production files. |
||
|
Michał Pierzchała
|
83322a3f2f |
test(layering): pin exact façade symbols for all workspace packages (#1574)
* test(layering): pin exact façade symbols for all workspace packages #1555 added the repo's first exact exported-symbol gate, pinning @agent-device/ad-replay's named export list. Every other workspace package was still covered only by the exports-subpath locks, which prove which files a package exposes but say nothing about what those files name — so any façade could grow a symbol silently. Pin all 29 exported subpaths across the remaining 8 packages: ad-script, contracts (14), kernel (8), maestro, provider-limrun, provider-webdriver, replay-test, and xml. The lists are the honest current surface, untrimmed — contracts/interaction alone names 140 symbols, and pinning the real number is what makes the next widening visible. The table is checked in both directions, so a new package or subpath that nobody pinned fails rather than being silently skipped. Pinning contracts needed the export-discovery helper widened: 13 of its 14 façades are bare `export * from '../x.ts'` barrels, and readNamedExports throws on those by design, because given only a source string the contributed set is genuinely unknowable. Given the FILE it is not, so readFacadeExports resolves the relative re-export chain and enumerates it. Resolution stays narrow — a package-specifier star still throws (that would mean re-entering another package's exports map, the unbounded widening the gate exists to refuse), cycles are visit-guarded, and a default export still throws through a barrel. Helper unit tests cover the shapes the merged AST scan handles but left unpinned: `export { default as x }` (the form between the two rejection rules — named, so reported, never `default`), a local `export { … }` list with no `from`, and multi-declarator `export const a = 1, b = 2`. Plant-verified per package rather than asserted: a stray export on ad-script, one two files deep behind contracts' `export *` chain, and an unpinned new subpath on xml each failed with a named diff; each reverted to green. Gates: check:layering (63 tests, up from 53) / typecheck / lint / format:check — green. * fix(layering): model real `export *` semantics; split the pinned table out Addresses both P1 findings on #1574. P1 — `readFacadeExports` did not model `export *` façade semantics. It unioned every child name and threw on every child default. Both are wrong: - Per GetExportedNames, a star export excludes the child's `default`, so a private `export default` in a leaf is not reachable through the barrel and does not widen the façade. It is now passed over rather than rejected; the previous test codified that false positive and is replaced. A default on the ENTRY file is still a real default export of the façade and still throws. - Per ResolveExport, a name two star sources resolve differently is `ambiguous` — importing it is a SyntaxError, so it is not part of the surface at all. Unioning would pin a symbol no consumer can import; ambiguity now throws and names both origins. Origins are tracked by declaring module rather than by path taken, so a diamond (two barrels reaching one declaration) resolves normally, and an explicit export shadows a star-provided name of the same name as the spec's own precedence does. Both counterfactuals are tested alongside the two rejection cases. P1 — module size. The 885-line generated FACADE_SYMBOLS table moves to a focused sibling, scripts/layering/facade-symbols.ts, leaving the behavioral tests at 642 lines (from 1,455) so the test file stays one bounded read per AGENTS.md. Gates: check:layering (66 tests, up from 63) / typecheck / lint / format:check — green. Contracts plant re-verified under the corrected semantics: a stray two files deep behind the `export *` chain still fails with a named diff, and reverts to green. * fix(layering): resolve re-export identity transitively; extract facade-exports Addresses both P1 findings on the second review round. P1 — named re-export identity stopped at the immediate source. Given `a` re-exporting `x` from `b`, `c` re-exporting `x` from `a`, and a façade starring both, ESM resolves ONE binding (b's `x`), but the walker identified the two paths as `b#x` and `a#x` and falsely rejected the façade as ambiguous. Reproduced before fixing. Origins now resolve through the chain to the binding a name ultimately names, by asking the child's own already-resolved map instead of synthesizing an identity from the specifier. A package specifier keeps a stable synthetic identity (it is not a file this gate reads), and a cycle in progress falls back to the immediate source. Two tests, counterfactual-verified against each other: the chain diamond now resolves to one name (confirmed failing with the old immediate-source identity, passing with the fix), and a same-depth chain whose branches bottom out in two genuinely distinct declarations still throws — so the fix cannot be satisfied by simply collapsing every duplicate. P1 — context-safety extraction was incomplete. Façade export enumeration moves to scripts/layering/facade-exports.ts (219 lines) with its own facade-exports.test.ts (245), registered in check:layering. package-boundaries.ts drops to 338 from 528 and its test file to 450 from 642: the boundary rules answer "may this file import that one?", this module answers "what does this façade name?". Every layering file is now under the 500-line tripwire except the generated symbol table, which the rule exempts. Gates: check:layering (68 tests, up from 66) / typecheck / lint / format:check — green. Contracts plant re-verified after the split. * fix(layering): filter `default` at the star, not at its source The reported P1 does not reproduce: intermediate `export { default } from './x.ts'` links are reported by oxc as kind `Name` with the name `default`, not kind `Default`, so they already resolve transitively; and for a terminal `export default <decl>`, the fallback identity `${child}#default` is exactly the canonical binding, so both paths agree. The exact five-module scenario from the review returns ['x']. That behavior is now pinned by a test so it cannot silently regress. Investigating it did surface a real spec violation in the opposite direction. Because a re-exported `default` is a named entry, it landed in the module's map and was then copied wholesale by star enumeration, so `export * from './mid.ts'` reported `default` as part of the surface — a name `GetExportedNames` explicitly skips, and which oxc itself labels `AllButDefault` on the star's own import. `default` is now filtered at the star rather than at the source. That placement is the point: the name has to stay in the module's map so a later `export { default as x }` can resolve its binding, while never being reachable through a star. Filtering at the source would have broken identity resolution — the very thing the review round before this one fixed. A façade entry re-exporting a default under the name `default` is now rejected too. It carries a default export exactly as `export default …` does; only the parse shape differs, and only the declared form was being caught. Three tests: the star filter (counterfactual-verified — removing the filter fails it — with a sibling name proving the module is still read), entry-level rejection, and the two-paths-to-one-default-binding case from the review. Gates: check:layering (71 tests, up from 68) / typecheck / lint / format:check — green. Contracts plant re-verified. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
761317deb7 |
refactor(daemon): extract native .ad replay to packages/ad-replay (#1478 P5) (#1555)
* refactor(replay): move the dependency-free engine leaves into packages/ad-replay Stage A of the #1478 P5 extraction: vars, plan-digest (+canonical-json, sole consumer), the target-identity classification core, report-action, and suggestion-ranking move verbatim; imports updated. The package facade temporarily re-exports the moved symbols so root consumers keep compiling; a later stage narrows it to inspectAdReplay/runAdReplay only. * chore(layering): register packages/ad-replay in the workspace and DAG * refactor(replay): define the three-operation replay selector port with dual adapters (#1478 P5) * refactor(daemon): route replay handlers through the selector port (#1478 P5) * refactor(replay): split target verification into engine policy and daemon authority (#1478 P5) * refactor(replay): move the .ad step loop behind inspectAdReplay/runAdReplay (#1478 P5) * refactor(replay): lock the ad-replay façade to its real consumers (#1478 P5) * test(replay): prove shared-id demotion on both selector-port adapters (#1555 review) * fix(replay): restore invalid replayBackend rejection on the native path (#1555 review) * refactor(replay): move shared .ad vocabulary to its owner, packages/ad-script (#1555 review) * refactor(replay): neutral step/run outcomes and digest/resume behind inspectAdReplay (#1555 review) P1 "do not smuggle daemon wire failures through a generic": drop the TResponse generic from AdReplayStepRuntime/runAdReplay. executeStep and handleActionFailure now return neutral tagged AdReplayStepOutcome/ AdReplayStepFailure values (kind/message/artifactPaths only); runAdReplay returns a neutral completed/failed AdReplayRunOutcome. The engine never holds or returns a DaemonResponse. The daemon adapter (createAdReplayStepRuntime, session-replay-runtime.ts) keeps its real wire response in a local side-map as it builds each neutral outcome, and runReplayScriptFile reads it back once runAdReplay reports which step failed, so the final response is byte-identical to before this split. P1 "parsing/planning/digest/resume must also occur behind runAdReplay": relocate computeReplayPlanDigest's call site and the --from/--plan-digest resume-point math (resolveReplayEntryIndex) behind inspectAdReplay's manifest as planDigest and a resolveEntryIndex closure. Neither is a new top-level export -- inspectAdReplay/runAdReplay stay the only two. Timing is preserved exactly (still called eagerly in prepareReplayPlan, before prepareReplaySession's coordinator-mutating side effects) since moving resume validation to run inside runAdReplay itself would let a rejected --from request mutate coordinator/session state first -- a real ordering hazard, not just a cosmetic one. computeReplayPlanDigest/ReplayPlanDigestMetadata/resolveReplayEntryIndex leave the ad-replay façade; request-router-repair-expired.test.ts and prepareReplayPlan read the digest/resume result off the manifest instead. * refactor(replay): relocate classifyTargetBindingMatch and pin the ad-replay façade (#1555 review) P1 "complete the binding façade instead of documenting deviations": classifyTargetBindingMatch never had a real consumer reachable through inspectAdReplay/runAdReplay -- both its callers (the daemon's record-time self-check in session-target-evidence.ts and its replay-time classification wrapper in session-replay-target-classification.ts) are daemon files that imported it directly. It interprets TargetAnnotationV1 evidence semantics shared beyond the engine, so it moves to packages/ad-script alongside target-annotation-identity.ts (new target-annotation-classification.ts + its test), and both daemon call sites now import it from there instead of @agent-device/ad-replay. One deviation remains and is reported rather than papered over per the review's own instruction: the four target-verification policy functions (planPreDispatchTargetVerification, planPostResolutionTargetVerification, deriveReplayTargetGuardMismatchEvidence, deriveWaitLandmarkMismatchEvidence) and the ReplaySelectorPort type family stay exported. Their sole caller, session-replay-target-verification.ts, interleaves these pure decisions with daemon-only async work (capture, SessionStore, coordinator/resume stamping, wire shaping) that must stay outside the engine by design; moving their call sites to live only behind runAdReplay would require restructuring that whole orchestration into new fine-grained AdReplayStepRuntime capabilities, which is out of scope for this pass. See packages/ad-replay/src/index.ts's header comment for the full reasoning. P1 "add the reviewer-required exact exported-symbol gate": adds readNamedExports (scripts/layering/package-boundaries.ts), a small parser over a façade's `export { .. } from`, `export type { .. } from`, and direct-declaration forms, and pins @agent-device/ad-replay's exact 21-symbol export list in package-boundaries.test.ts. Plant-verified: a stray `export const` addition failed the assertion; removed it and the gate went green again. * refactor(replay): drive target verification from the engine step loop (#1555 review) Moves the verify-then-dispatch decision flow into packages/ad-replay's step loop so the four target-verification policy functions (plan{PostResolution,PreDispatch}TargetVerification, derive{ReplayTargetGuardMismatch,WaitLandmark}MismatchEvidence) become engine-private and leave the ad-replay façade. The daemon (session-replay-target-verification.ts) shrinks to the narrow AdReplayStepRuntime capabilities the engine drives: routing (beginTargetVerification), capture (captureObservation), classification (classifyTarget), dispatch (dispatchStep), and wire-building (buildRecordedUnverifiableFailure, buildTargetBindingFailure, buildPostDispatchTargetBindingFailure). Wire output and replay-compat stay byte-identical; the exact-symbol façade gate is updated to the shrunken export list. * refactor(daemon): decompose the replay adapter's two over-threshold functions (#1555) * refactor(replay): fold #1554's keep-session terminal-lifecycle policy into the ad-replay engine Rebasing p5/extract-ad-replay onto main pulled in #1554's --keep-session feature, which had grown its own daemon-side terminal-close-suppression predicate (session-replay-terminal-lifecycle.ts's resolveSuppressedTerminalCloseIndex/countExecutedReplayActions) independently of this branch's own engine-side one (step-loop.ts's isRepairArmedTerminalCloseAction). Both are the same decision family — replay --keep-session and an active --save-script repair now share ONE structural resolution (resolveSuppressedTerminalCloseIndex, generalized to "terminal among EXECUTABLE actions" rather than the old physical-last-index check) and one suppression check inside runAdReplay, gated on keepSession OR runtime.isRepairArmed(). AdReplayRunRequest grew a keepSession field; the neutral 'replayed' count in AdReplayRunOutcome is now computed inline in the loop instead of the daemon's old actions.length - entryIndex approximation. requireLiveSessionForKeepSession (the --keep-session live-session postcondition) stays daemon-side, inlined into session-replay-runtime.ts, since it inspects SessionStore state the engine never sees. The daemon-only session-replay-terminal-lifecycle.ts this arrived with is deleted entirely — its isExecutableReplayAction was a duplicate of the engine's own. runReplayScriptFile's Maestro-format routing (including the new --keep-session Maestro rejection) was extracted into routeMaestroReplay to keep the function under fallow's complexity threshold after re-threading keepSession through it. Added packages/ad-replay/src/internal/__tests__/step-loop.test.ts covering the unified suppression decision (both keepSession and repair-armed) directly against runAdReplay, including the terminal-among-executable-actions case with a trailing nested replay marker. The daemon-level integration tests (6 tests in session-replay-terminal-lifecycle.test.ts, exercising the same behavior through runReplayScriptFile) and the SDK provider-scenario test (active-session-script-publication.test.ts) needed no changes and pass unmodified. * refactor(daemon): decompose session-replay-runtime.ts into three modules (#1555) Splits the ~1096-line replay runtime into cohesive pieces, keeping session-replay-runtime.ts as thin orchestration (~240 LOC): - session-replay-runtime-engine-adapter.ts: the AdReplayStepRuntime adapter (createAdReplayStepRuntime, the build*Failure capability implementations, and the lastResponse/lastObservation side-map mechanics), extracted verbatim. - session-replay-runtime-plan.ts: extended with the plan-side helpers (validateReplayBackendFlag, inspectReplayPlanManifest, resolveReplayPlanEntryIndex, prepareReplayPlan, routeMaestroReplay) alongside the buildReplayMetadataFlags helper already there — buildReplayMetadataFlags is now module-private since its one caller moved into the same file. Also introduces ReplayScriptFileParams, named here (instead of derived via Parameters<typeof runReplayScriptFile>) so routeMaestroReplay can reference the shape without importing back from session-replay-runtime.ts. - session-replay-runtime-session.ts (new): session preparation (prepareReplaySession and its coordinator arming/repair-preflight helpers), extracted verbatim. Coordinator ownership is unchanged: createReplayCoordinator is still constructed only in session-replay-runtime.ts, matching replay-coordinator-ownership.test.ts's allowlist as-is — every extracted module receives the already-constructed ReplayCoordinator as a parameter. Pure move; no behavior change. * test(replay): cover pre-step artifact ordering and resume-before-mutation (#1555) Two invariants found during the P5 decomposition pass now have direct counterfactual-verified coverage: - packages/ad-replay/src/internal/__tests__/step-loop.test.ts: a post-dispatch target-binding mismatch (dispatchWithGuard) must report the accumulated PRE-step artifact snapshot it was called with, never the artifacts the failed dispatch itself produced. Verified red by swapping the buildPostDispatchTargetBindingFailure call to outcome.artifactPaths. - src/daemon/handlers/__tests__/session-replay-runtime-plan.test.ts: a rejected --from/--plan-digest resume must never reach prepareReplaySession's coordinator-mutating writes (the R2 ordering invariant) — a pre-armed repair transaction and corrective-resume watermark are asserted byte-for-byte unchanged after rejection. Verified red by calling prepareReplaySession before honoring the plan-validation rejection. * fix(ad-replay): enforce the exact two-entrypoint facade (#1555 review P1) packages/ad-replay/src/index.ts now exports exactly two value symbols, inspectAdReplay and runAdReplay, and zero types — formatReplaySuccessMessage (presentation) moves beside its one caller in session-replay-runtime.ts, and every type a root daemon file needs is derived structurally off the two entrypoints in the one new src/daemon/ad-replay-facade-types.ts module instead of being named off the façade. scripts/layering/package-boundaries.ts's readNamedExports is rewritten on oxc-parser's own static-export table instead of a regex, so it can no longer silently miss a widening export form: a bare `export *` re-export or an `export default` now throws (an un-enumerable, and therefore un-pinnable, export), while `export * as ns` and every other enumerable form is still counted. The pinned exact-symbol assertion in package-boundaries.test.ts is narrowed to ['inspectAdReplay', 'runAdReplay']. * fix(ad-replay): translate wire failures before the engine boundary (#1555 review P1) AdReplayDispatchOutcome's guard-mismatch/landmark-mismatch variants carried a generic `details: Record<string, unknown> | undefined` bag straight off the wire response — a daemon wire projection crossing into the engine even though the outcome itself was already a neutral type. The daemon adapter (session-replay-runtime-engine-adapter.ts) now narrows that bag into the typed AdReplayGuardMismatchEvidence/AdReplayLandmarkMismatchEvidence shapes (observed identity, expected/observed structural denotation, ancestry entries, match count) before returning the outcome; the unknown-parsing readers move there with the wire-reading responsibility they always were. target-verification.ts's deriveReplayTargetGuardMismatchEvidence/ deriveWaitLandmarkMismatchEvidence now consume only the typed values — no `unknown`-valued record type remains on any engine-crossing signature. * fix(ad-replay): move variable semantics/planning behind runAdReplay (#1555 review P1) The daemon assembled the `${VAR}` scope (buildPreparedReplayScope) and interpolated actions at two independent call sites: dispatch's own (invokeReplayAction) and target verification's separate one (resolveTargetVerificationEntry) — duplicated orchestration the P5 design assigns to the engine. runAdReplay's request now carries the raw scope INPUTS (varSources: plain builtins/file/shell/cli-env data, plus actionLines/actionSourcePaths/ resolvedPath for interpolation-error location) instead of a built scope; the engine builds the scope and resolves each action exactly once per step, handing the RESOLVED action to dispatchStep/beginTargetVerification while every other capability still receives the ORIGINAL recorded action (a target-binding divergence reports the recorded selector, never an expanded ${VAR}). This is the one resolution site now — session-replay-action-runtime.ts's invokeReplayAction and session-replay-target-verification.ts's resolveTargetVerificationEntry no longer hold a scope or call resolveReplayAction themselves. Scrub-value collection (collectReplayScrubbableVarValues, for divergence-report redaction) is kept single-sourced in the engine too: it's computed from the engine's own live scope and threaded to each build-failure/handleActionFailure capability as an explicit scrubVars argument, rather than the daemon recomputing it from a second scope object (which would have gone stale, since expandedBuiltinNames tracking now only happens engine-side). The Maestro replay path's own daemon-side vars usage is unrelated (a different engine) and is out of scope here. * fix(ad-script): make ${VAR} interpolation a linear scanner CodeQL flagged the interpolation regex's fallback group as js/polynomial-redos once vars.ts moved into packages/ (library-input classification): every ${NAME:- prefix of an unclosed input rescanned to end-of-string, quadratic overall — 1,857 ms measured on 20k repetitions of '${A:-['. Replaced with a single-pass scanner; failed fallback scans emit their span verbatim and resume after it (escape-pair alignment is identical from every candidate start inside the span, so no later candidate can terminate where the failed scan could not). Equivalence: 200k-trial differential fuzz against the retired regex over the adversarial alphabet, zero mismatches; both adversarial shapes now resolve in 1-2 ms. * refactor(ad-replay): typed façade replaces the zero-type rule (#1555 structural-quality review) Reverses the exact-two-value zero-type export shape #1555's second review pass established: it forced every root type derivation through one shim (src/daemon/ad-replay-facade-types.ts) and left four daemon-side twin types (TargetVerificationEntry, TargetClassificationOutcome, TargetBindingFailureEvidence, ReplayVerifiedTargetGuard) plus a toDaemonEvidence copy translator shadowing the engine's own shapes. packages/ad-replay/src/index.ts now exports inspectAdReplay/runAdReplay (unchanged, still the only two values) plus the neutral vocabulary their signatures are built from, by name — following packages/maestro's façade precedent. The exact-symbol gate in scripts/layering/package-boundaries.test.ts is widened to pin the full sorted list (values + types). The four daemon twins are deleted; session-replay-target-verification.ts and session-replay-runtime-engine-adapter.ts now use the engine's own AdReplayVerificationEntry/AdReplayTargetClassification/ AdReplayTargetBindingEvidence/AdReplayVerifiedTargetGuard directly. TargetBindingDivergenceBuilt's array fields are now readonly-compatible, so toDaemonEvidence's copy is gone — evidence flows through unchanged. * fix(ad-replay): honor the selector port's own contract in the parse gate target-verification.ts's planPreDispatchTargetVerification used resolveRecordedTarget (operation 2, resolve) over an empty node tree purely to read its parse-invalid reason — a resolve call standing in for a parse call, even though readSelectorExpression (operation 1, parse) exists to answer exactly that question and was already unused inside the engine. Replaced with port.readSelectorExpression('ordinary', [token]). The mapping is not 'invalid' -> skip: production's 'ordinary'/'wait' grammars only ever record a boundary once it has already parsed, so a single malformed token can only come back 'not-applicable' there ('invalid' is unreachable from this call site on the production adapter). Both non-'expression' outcomes map to skip, matching the historical behavior (a single parse-invalid reason covered both cases). platform dropped from the function's params — it was only ever threaded to the resolve call this replaces. Added a contract-suite cell pinning the exact (diverging) discriminant each adapter reports for a selector-shaped-but-malformed bare token, and why the divergence is harmless for the one real consumer. * refactor(ad-replay): split step-loop.ts and shrink the daemon adapter (#1555 structural-quality review) step-loop.ts (810 LOC) splits three ways, following packages/maestro's own precedent: - internal/runtime-port-types.ts: the AdReplayStepRuntime boundary vocabulary (all the neutral types the engine/daemon exchange). - internal/verify-dispatch.ts: verifyAndDispatchStep + its dispatchNoGuard/ dispatchWithGuard helpers. - internal/step-loop.ts: runAdReplay itself plus the terminal-close/ executable-action structural logic (isExecutableReplayAction, resolveSuppressedTerminalCloseIndex). packages/ad-replay/src/index.ts's type exports now source from runtime-port-types.ts. step-loop.test.ts's AdReplayStepRuntime import moves to the new path (no assertion changes). src/daemon/handlers/session-replay-runtime-engine-adapter.ts (553 LOC after item 1's twin removal) shrinks to 294 via two further extractions: - session-replay-dispatch-narrowing.ts: the wire `details` bag -> typed evidence narrowing and dispatch-failure classification. - session-replay-runtime-step-support.ts: ReplayStepContext (moved here to avoid a cycle with the adapter, which re-exports it by name) plus the failure-wrapping/diagnostics-support helpers. Final LOC: adapter 294, dispatch-narrowing 148, step-support 153, step-loop 225, verify-dispatch 246, runtime-port-types 374. * test(ad-replay): package-local tests for resume.ts/target-verification.ts + terminal-lifecycle test rename resume.test.ts covers resolveReplayEntryIndex directly (previously only exercised transitively through the daemon's session-replay-runtime-plan tests): no --from/--plan-digest, the paired-flags requirement, in-range --from, out-of-range rejection, stale-digest rejection, the authorized empty-tail boundary (actionCount + 1) gated on a matching watermark, and the unperformed-record-and-heal growth check. Counterfactual run and restored: widening describeOutOfRangeResumeFrom's bound turns the out-of-range/ empty-tail-without-watermark assertions red (2 failures observed). target-verification.test.ts covers all four engine policy functions directly: the two plan* pre-capture gates and the two derive* post-dispatch evidence builders, including item 2's own new decision surface (a fake ReplaySelectorPort proving both non-'expression' readSelectorExpression outcomes map to skip). Counterfactual run and restored: narrowing the check to the literal `'invalid' -> skip` reading turns the 'not-applicable' case red (reports recorded-unverifiable instead of skip). session-replay-terminal-lifecycle.test.ts renamed to session-replay-runtime-keep-session.test.ts: its production module (session-replay-terminal-lifecycle.ts) was already deleted by the #1554 fold-in, and its six cases drive the full runReplayScriptFile round trip against a real SessionStore (including daemon-only postconditions the engine's step loop never reaches) rather than testing engine policy through the façade in isolation — the engine's own terminal-close-suppression decision already has direct, cheaper coverage in step-loop.test.ts. No assertion changes; both files' header comments cross-reference the split. * refactor(ad-replay): compute scrub values once per step, one name end to end collectReplayScrubbableVarValues(scope) was called fresh at 5 separate return points inside one verifyAndDispatchStep invocation plus once more in handleActionFailure — always the same result, since nothing between them mutates scope. step-loop.ts's runAdReplay now computes scrubVars ONCE per step, right after resolveReplayAction (the one call that can grow the scope's expanded-builtins set), and threads it as a plain readonly AdReplayScrubValue[] value; verify-dispatch.ts no longer imports ReplayVarScope or collectReplayScrubbableVarValues at all. "One name" end to end: the daemon's TargetBindingDivergenceContext.scrubVars and withReplayFailureDiagnostics's scrubVars param used a separately-derived ReturnType<typeof collectReplayScrubbableVarValues> (mutable array) instead of the engine's own AdReplayScrubValue, requiring a [...scrubVars] copy at every daemon call site to satisfy the mutable-array type. Both now use readonly AdReplayScrubValue[]/readonly ReplayVarScrubEntry[] (structurally identical, already readonly-safe downstream — scrubReplayVarValues and createReplayDivergenceSanitizer already accepted readonly arrays), so the four [...scrubVars] copies in session-replay-runtime-engine-adapter.ts are gone. * fix(daemon): make lastObservation genuinely per-step, not per-run createAdReplayStepRuntime's lastObservation closure lives for the whole replay run (one factory call covers every step), but was never reset between steps. Every current buildTargetBindingFailure call site happens to be preceded by this same step's own captureObservation, so the `lastObservation ?? { reason: 'observation-missing' }` fallback could never actually fire — but if it ever did (a future call path reaching buildTargetBindingFailure without capturing first), it would silently attach the PREVIOUS step's screen instead of reporting the missing-capture condition the fallback message claims. armStep runs exactly once per step, before any of that step's other capabilities (verified against step-loop.ts's runAdReplay loop order) — the natural per-step boundary. It now clears lastObservation first. No behavior change on any reachable path today (full daemon + ad-replay suite: 1766/1766 green); an unrelated device-claim-prune contention flake was observed once and did not reproduce on isolated or full-suite reruns. * docs(ad-replay): fix decayed review-changelog comments naming defunct symbols Four comments named symbols/paths that no longer exist, left behind by earlier review passes describing PR history rather than the current constraint: - session-replay-runtime-step-support.ts / session-replay-runtime.ts (2 sites): referenced a function called executeStep, which was never reintroduced under that name after the P5 split — the actual mechanism is the runtime's dispatch/build-failure capabilities recording into the lastResponse side-map. - session-replay-runtime.ts: referenced an engine collectArtifactPaths capability that does not exist — artifactPaths is a daemon-side Set the adapter mutates via collectReplayActionArtifactPaths. - packages/ad-replay/src/internal/selector-port.ts: pointed at ./testing/in-memory-selector-port.ts, the in-memory adapter's pre-stage-D location — it has lived at src/__tests__/test-utils/in-memory-replay-selector-port.ts since. - session-replay-repair-hint.ts / session-replay-runtime-step-support.ts (2 sites): named target-identity.ts, which does not exist (the real file is target-identity-node.ts); the second site additionally mislabeled classifyReplayTarget as engine-side when it is daemon-side (session-replay-target-classification.ts). Comment-only; no behavior change. * refactor(ad-script): move declaredScriptPlatform to its natural shared owner packages/ad-replay/src/internal/inspect.ts's declaredScriptPlatform and src/daemon/replay-device-selection.ts's readScriptReplaySelection each kept their own copy of the same "platform declared before the first open" scan over runtime/open actions — .ad script semantics, not engine or daemon policy, needed independently by ad-replay's plan-digest precedence and the daemon's device-selection platform resolution. Verified this was a genuine duplicate (not the single-sourced state I initially reported): readScriptReplaySelection's platform-tracking loop computes the identical result via a differently-shaped traversal fused with its own app-target scan. resolveDeclaredScriptPlatform now lives in packages/ad-script (its natural owner: the one package both ad-replay and the daemon already depend on, avoiding the R11 issue that justified the original duplication). The daemon's app-target scan stays its own separate pass; fusing it back into the shared function would smuggle a daemon-only concern into ad-script for no measurable cost (the actions array is small, and the shared function already stops at the same point the app-target scan needs to look). * docs(ad-replay): fix package.json description to match the current façade Described "target-identity, variable substitution, plan-digest, and report primitives" — the wide pre-#1555-review façade shape. Vars/identity/report vocabulary moved to ad-script/daemon across the P5 and #1555 review passes; the package now exports exactly inspectAdReplay/runAdReplay plus the neutral AdReplayStepRuntime vocabulary. Description updated to match. * refactor(daemon): fold the step-support fragment back into the engine adapter A simplicity audit judged session-replay-runtime-step-support.ts a size-target fragment, not a concern boundary: four unrelated concerns, one consumer, and a header comment admitting it existed to satisfy the <300 LOC metric. Folded back; the previously-exported helpers are module-private again; the adapter's honest size is renegotiated from the plan metric (dispatch-narrowing stays extracted — it has one nameable job). |
||
|
Michał Pierzchała
|
b9509fe006 |
refactor: extract the .ad script codec into packages/ad-script (#1478) (#1536)
* refactor: extract the .ad script codec into packages/ad-script Moves the mutually-coupled .ad read/write codec (script.ts, script-utils.ts, script-formatting.ts, open-script.ts) plus the target-v1 annotation SERDE slice of target-identity.ts into a new private leaf package, @agent-device/ad-script, exporting only `.`. This is option 1 from the P5 scoping dossier on #1478: the codec is shared by the daemon's session-script publication writer, the future replay engine, the CLI's `replay export`, and Maestro's failure-label formatting, so it can no longer live in root src/ once packages/ad-replay lands (R11 forbids a package reaching into root src), and a second export subpath or writer-half duplication are both ruled out by existing gates/tests. target-identity.ts keeps only the record/replay-shared classification core (classifyTargetBindingMatch, local-identity/ancestry-prefix matching), importing its shared types from the new package. Every real consumer (re-derived by grep, not the dossier's list alone) is rewired to @agent-device/ad-script. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> * refactor: trim the ad-script façade to real consumers, lock the one-export boundary - packages/ad-script/src/index.ts: drop parseReplaySeriesFlags, formatTargetAnnotationCommentLine, parseTargetAnnotationCommentLine, TargetAnnotationLineParseResult, and TargetRect from the public façade — none has a consumer outside the package (re-swept every remaining export by grep; everything else kept has at least one real external importer). The functions/types stay exported from their declaring internal modules for the package's own internal use (script.ts, script-formatting.ts). - scripts/layering/package-boundaries.test.ts: add the parallel R11 assertions "the real tree parses, declares, and passes R11" already makes for maestro/provider-webdriver/provider-limrun/xml — ad-script exports exactly `.`, depends on exactly contracts+kernel, and is declared in root package.json — plus ad-script entries in the deep-resolution rejection coverage. Verified the lock catches a regression: temporarily added a fake `./codec` export to packages/ad-script/package.json and confirmed both the export-key-list assertion and the deep-resolution-rejection assertion fail; removed the plant and reconfirmed green. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> * fix: remove polynomial-redos ambiguity from the target-v1 annotation line regex CodeQL js/polynomial-redos flagged TARGET_ANNOTATION_LINE_RE (packages/ad-script/src/internal/target-annotation-serde.ts): the payload group's `\s+(.*)` let `\s+` and the unconstrained `.*` both match whitespace, so a run of separator whitespace that ultimately fails to complete the match has many `\s+`/`.*` splits to backtrack through before concluding failure. Anchor the payload group on `\S` (the exact complement of `\s`), so the mandatory `\s+` separator and the payload's first character can never overlap — the split point becomes unique and no backtracking is possible. Behavior-preserving: the only caller (parseTargetAnnotationCommentLine) always matches against an already-.trim()-ed line, whose last character (whenever the tag matches at all) is never whitespace — so a payload section `\S.*` would reject (content that is entirely whitespace) can never reach this regex through the real call path. Verified against the frozen replay-compat corpus and the full serde/parser test suites, unmodified. Added a regression test with the exact adversarial shape CodeQL/the reviewer cited (many tab pairs after the version digits), asserting sub-second parse. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> * test(ad-script): pin the annotation-line pattern's linear rejection directly The entry-point adversarial case matched greedily even with the retired regex (trim strips edge whitespace and per-line input carries no newline), so it proved nothing about the pattern. The regression surface is the pattern itself: an interior tab run with an x-newline tail fails the match, which the retired form re-split quadratically (3.7s at 100k tabs) and the \S anchor rejects in one attempt. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
da93191201 |
refactor: move Limrun provider behind package facade (#1518)
* refactor: move Limrun provider behind package facade * fix: preserve Limrun public provider types * fix: tighten Limrun provider facade boundaries * test: harden Limrun compatibility coverage * fix: narrow Limrun public type exports * fix: narrow Limrun provider exports |
||
|
Michał Pierzchała
|
b125435989 |
refactor: extract WebDriver provider package (#1504)
* refactor: extract webdriver provider package * refactor: consolidate shared XML codec |
||
|
Michał Pierzchała
|
a3ab69a110 |
refactor(replay-test): neutralize the values crossing the scheduler seam (#1478 P3, part 1) (#1509)
* refactor(replay-test): neutralize the values crossing the scheduler seam
#1478 P3, part 1 of 2. Prepares the replay-test extraction by removing every
non-neutral value that crosses the scheduler seam, in place under `src/`, so the
physical move to `packages/replay-test` is a file move rather than a redesign.
`DaemonResponse` no longer crosses the seam. `session-test-types.ts` typed
`runReplay`/`finalizeAttempt` as returning a daemon response and the scheduler read
`.error.code`, `.error.details`, and `.data.replayed/.healed/.warnings/
.snapshotDiagnostics` off it throughout. That is invisible to R10 today only
because `checkDaemonTypesImporters` skips `src/daemon/`; once the files live in a
package they become external `daemon/types.ts` importers, which the ratchet only
lets shrink. Attempts now resolve as tagged `ReplayTestAttemptOutcome` values
carrying exactly what the scheduler consumes, including an `infrastructure` tag —
classifying an environmental failure needs platform boot-diagnostic vocabulary the
scheduler must not import, so the host decides and the scheduler reads the verdict.
`session-test-outcome.ts` is the one place a daemon response becomes an outcome.
Step events get a narrow per-attempt port. They were emitted from
`session-replay-runtime.ts` and `session-replay-maestro-observer.ts`, both reading
a request-global `AsyncLocalStorage` seeded per attempt. The scheduler now hands
each attempt an `onStep` sink, threaded the way `tracePath` already is; both
engines call it and `withReplayTestActionProgress`/`readReplayTestActionProgress`
are gone. A direct `replay` simply has no sink.
ADR 0012 divergence becomes a neutral leaf. `src/replay/divergence.ts` depended
only on kernel contracts and redaction, yet Maestro constructs divergences too and
CLI/MCP both render them, so P5 could not have moved it into `packages/ad-replay`.
It is now `@agent-device/contracts/divergence`; the renderer's output text is
unchanged.
The progress wire vocabulary moves to `@agent-device/contracts/progress`. It is
serialized by `request-progress-protocol.ts` and reconstructed by the CLI reporter
path, so it belongs below both; `src/request/progress.ts` keeps only the sink and
its AsyncLocalStorage binding.
Together these clear all four of replay-test's recorded R10 migration imports, so
the rule now enforces unconditionally for that module.
Behavior is unchanged. The shipped reporter contract — export spellings,
object/factory loading, hook names, timing/order, value fields, the synchronous
live-hook rule, awaited suite completion, error handling, exit codes — is
untouched, and `session-test-reporter-values.test.ts` passes unmodified. The
`--shard-all` `total`/`runnable` asymmetry is preserved as characterized.
Refs #1478
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8
* test(replay-test): pin the Maestro reporter step path against the onStep port
Review finding on #1509: the native `.ad` reporter ratchet exercises only one of
the two `onStep` forwarding chains, so deleting a link in the Maestro chain would
silently stop `onTestStep` for every `test --maestro` run while every existing
reporter test stayed green. Same defect class as the dropped diagnosticId/logPath
(#1501) and the dropped reporter `hint` (#1505).
Maestro is one of P3's two required real adapters and its chain shares no links
with the native one below `runReplayScriptFile`:
scheduler sink -> runReplayScriptFile -> runTypedMaestroReplayFile
-> createMaestroReplayObserver({ onStep }) -> actionStarted -> onStep
Adds a Maestro scenario driving `test --maestro` through the real session handler
and the real reporter registry. It asserts the step payload the engine produces
(`stepIndex`/`stepTotal`/`stepCommand`/`stepValue`, including that a value-less
command stays value-less) together with the attempt/session identity the scheduler
supplies, since that half of the event came from request-global AsyncLocalStorage
before P3. A second case drives a retry so step events must carry attempt-1's
session and then attempt-2's. The flow `name` also pins the reporter `title`, a
value only the Maestro path can produce.
New file rather than an addition to session-test-reporter-values.test.ts: that file
is the pinned characterization and must keep passing unmodified, and Maestro needs
its own vi.mock of core/dispatch for device resolution.
Counterfactual run, both links, each restored after:
- dropping `onStep` from createMaestroReplayObserver in
session-replay-maestro-runtime.ts
- dropping the emitMaestroStep call from actionStarted in
session-replay-maestro-observer.ts
Each dropped both onTestStep events ("expected [ 'onSuiteStart', 'onTestStart',
…(2) ] to deeply equal [ 'onSuiteStart', 'onTestStart', …(4) ]") and failed both
new cases, while session-test-reporter-values.test.ts passed all 4 — exactly the
hole the reviewer identified.
Test-only; no production change. Bundle output is byte-identical to
|
||
|
Michał Pierzchała
|
0e51007b04 |
refactor: isolate maestro engine package (#1506)
* refactor: isolate maestro engine package * perf: deepen maestro facade boundaries |
||
|
Michał Pierzchała
|
0ee2a86129 |
refactor: extract contracts workspace package (#1499)
* refactor: extract contracts workspace package * fix: preserve screenshot diff result contract * test: stabilize Android keyboard smoke |
||
|
Michał Pierzchała
|
76453add71 |
refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) (#1494)
* refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) Extend the workspace with packages/* and move the kernel behind an enforced public API: packages/kernel with nine consumer-earned subpath exports (errors, device, snapshot, contracts, collections, rect, redaction, daemon-error, bounds — the last absorbed from utils as Rect vocabulary). Every kernel import repo-wide becomes the @agent-device/kernel/<sub> specifier; kernel tests move to src/__tests__/kernel/ and exercise the package surface. The root declares the package in devDependencies (workspace:*), tsdown bundles it (noExternal) so the published artifact and its runtime dependency manifest are unchanged. Gate rewiring in the same change, per the W0 brief: - R1 kernel-sink retires (physically subsumed); new R11 package-boundaries guards no-root-back-imports, relative tunnelling past exports maps, undeclared workspace deps, and non-exported subpaths, with runtime resolution pins via import.meta.resolve. - resolveImportEdges and mutation ownership follow workspace specifiers through exports maps, keeping R4 cycle checks, depgraph, and derived test ownership connected across the seam (kernel-errors still owns 495 tests). listSourceFiles includes packages/*/src. - kernel becomes an unranked zone; mutation registry, stryker mutate globs, and the mutation-affected workflow path filter move to packages/kernel/src/errors.ts. - check:affected gains packages/ ownership (manifests fail open); vitest and coverage include packages/*/src; fallow ignores packages/** (its resolver cannot follow workspace specifiers). - The affected-selector CI job installs dependencies: its closure now crosses workspace specifiers, and the R8 relative exception is unsafe for production src files (Node ESM does not realpath, so dual specifier/relative loads would instantiate modules twice). The R8 zero-dep set is pinned empty with that rationale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: address W0 review — mutation sandbox, exports-map resolution, tsc -b Review findings on #1494, all five: 1. contracts-schema-public.test.ts reads the kernel source at its packages/ path (fs access invisible to the codemod and typecheck). 2. Mutation lane: Stryker sandboxes the tree but pnpm's node_modules symlink resolves @agent-device/* back to the real repo, so mutants in the sandbox never load and vitest.related finds no tests. vitest.mutation.config.ts now aliases each EXPORTED specifier to its source (derived from exports maps, never a wildcard), keeping resolution inside the mutated tree. Validated: kernel-errors module runs end to end (dry run 3,984 tests, mutants killed, exit 0). 3. Layering/depgraph resolve workspace specifiers through the exports-derived map (workspaceSpecifierTargets) instead of reconstructing paths, so '.'-facade packages resolve; the positional fallback remains only for map-less fixtures (P0 pin). 4. Per-package project references implemented: packages/kernel is composite (emitDeclarationOnly -> dist-types, gitignored), the root references it, and typecheck becomes tsc -b — probed to catch type errors on both sides under TypeScript 7 native. 5. R11's relative-route exception now requires membership in an actual R8 zero-dep job closure (zeroDepClosureFiles walks entries), not mere scripts/ placement — closing the dual-instantiation bypass. Also from review discussion: daemon-error moves out of the kernel package to src/client/ — its consumers (cli, client facade) rehydrate wire DaemonErrors client-side; the daemon only produces them. Kernel drops to 8 exported subpaths before any of them ship. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * refactor: one exports-map reader for mutation alias and ownership Fallow flagged workspaceExportAliases (cognitive 15, CRAP 90). The manifest-reading logic already exists as workspaceSpecifierTargets in scripts/layering/package-boundaries.ts, so both the Stryker sandbox alias table and the mutation ownership walker now consume it instead of carrying near-clones. Behavior unchanged; mutation suite 45/45 and changed-code fallow green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: composite kernel without a root references edge FreeRange runs plain `tsc -p tsconfig.json`, and a root `references` entry makes non-build-mode TypeScript demand the referenced project's built declarations (TS6305) — a standing "build first" tax on every plain -p consumer (fr, editors). Keep the per-package composite project and build it in typecheck (`tsc -b packages/kernel` before the root and examples/sdk passes), but drop the root references edge: root consumption resolves through exports to source, identical to runtime and to the bundler. Probed: plain -p green with no prebuilt output; kernel-side type errors still caught by its own build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: R11 uses the layering parser; mutation config is a fallow entry Review blockers on #1494: - R11's private single-quote regex could miss a double-quoted or re-export route into packages/*/src. specifierSites now delegates to the layering model's parseImports (both quote styles, side-effect imports, re-exports, dynamic imports), with direct regressions for each formerly-invisible form. - vitest.mutation.config.ts becomes a declared fallow entry instead of a tolerated unused-file finding: the full-repo audit now reports it reachable (unused files 2 -> 1; the remainder predates this PR). FreeRange clean-checkout evidence: with packages/kernel/dist-types and every *.tsbuildinfo deleted, `pnpm check:freerange` reports 0 findings on this head — the TS6305 topology died with the root references edge in the previous commit; check:freerange has no build precondition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep --------- Co-authored-by: Claude <noreply@anthropic.com> |