mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
fix/web-snapshot-disabled-annotation
233 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michał Pierzchała
|
5676d5ff8a |
refactor(apple): drop dead runner code and collapse duplicated helpers (#1936)
* refactor(apple): drop dead runner code and collapse duplicated helpers Removes declarations with no consumers (findScopeElement, interactiveTypes, two unused PresentedNode convenience inits) and collapses copy-pasted logic: DataPayload now relies on the synthesized memberwise init, TvRemoteButton is String-raw-valued, point-hit sorting shares smallestElementFirst, command-id trim-or-nil lives once on RunnerCommandJournal, scroll/desktopScroll share direction and durationMs validators, and the seven inline NSError refusals use unsupportedOperationError. elementTypeName reads a table pinned by the visibility-fold parity test. The packager now skips files whose unit-test blocks were their whole body, so 10 test-only files stop shipping (and stop compiling on user machines) as empty translation units. Packaged Swift: 432.3 kB -> 427.8 kB; 56 files instead of 66. Net -144 lines. * test(apple): pin skeleton-file exclusion in the packaging guard The strip fixture always kept runtime content, so reverting the skeleton-skip branch left every guard green. The new fixture's whole body is unit-test blocks; the packaged path must be absent while a non-skeleton sibling still ships. Observed red with the skip branch disabled before re-enabling it. * refactor(apple): tighten runner cleanup boundaries |
||
|
Michał Pierzchała
|
30de1597d3 |
ci: attribute native package size and trim Apple runner (#1934)
* ci: attribute npm package size by shipped component * refactor: modularize size reporting and trim Apple runner * ci: preserve size reporter modules across base checkout |
||
|
Michał Pierzchała
|
d57aa69777 |
test: add macOS platform command coverage manifest (#1922)
* test: add macOS platform command coverage manifest * fix: remove unused macOS coverage type exports * test: route macOS coverage away from iOS lane * fix: account for host-dependent macOS audio capability * fix: run macOS coverage manifest in CI |
||
|
Michał Pierzchała
|
46eff36f85 |
refactor: migrate focus to the request-bound device runtime (#1925)
* refactor: migrate focus to the request-bound device runtime Wave 5's first unit (#1739, ADR 0019). `focus x y` and `find <q> focus` now reach the device through one admitted, request-bound `focusPoint` operation instead of the `handleFocusCommand` interactor leaf and its dispatch-table arm. - New `FocusRuntimeOperations` contract with local and provider interactor binders, mirroring the screenshot/element-text seam rather than inventing a second way for one operation class to reach its mechanics. - Exact-owner facts replace the capability bucket: apple simulator/device, android emulator/device/unknown, harmonyos emulator/device, linux device, web device, vega none, providers wherever their interactor is reachable. That is the retired bucket's cell table, restated as facts. - `focus` leaves BASE_COMMAND_CAPABILITY_MATRIX and both hand-maintained overlays (HARMONYOS_SUPPORTED_COMMANDS, WEB_INTERACTION_COMMANDS). - R40 is the new parametrized cutover row; `focusPoint` has exactly one owner. - The `x y` positional parse moves to utils and is shared with the still-legacy touch siblings, so a migrated command cannot drift from them. `find` stays legacy: this unit owns its focus leg only, its `type` leg still dispatches, and R35 waits on the Wave 5 `type` unit. * test(focus): cover the owning interactor binders, lower the find ratchet Review follow-ups on #1925. P1: focus-runtime.test.ts bound a fake focusPoint, so deleting the interactor call inside bindLocalFocusInteractor left focus a successful no-op with every test green. Adds packages/contracts/src/focus-runtime.test.ts, which executes both binders and asserts resolver context, positional (x, y) forwarding, the structured missing-provider failure, and that an already-cancelled request never resolves an interactor at all. Two planted mutants confirm it bites: removing `await interactor.focus(input.point.x, input.point.y)` and transposing its two arguments each fail exactly the two forwarding tests, while the daemon-level focus and find suites stay green — which is the gap the reviewer named. Coverage: find.test.ts shrank to 1204 lines when its focus assertion moved off the dispatch mock; the ratchet pin follows it down. * test(focus): add live Linux focus coverage to the desktop replay The Linux `focus` claim rested on the provider scenario at command-contract level. The desktop replay runs on real Linux hardware in the Smoke lane, so it now runs a coordinate focus and re-asserts the session survived it. Coordinate, not selector: the step exists to prove the migrated `focusPoint` path executes on real hardware, so it must not be able to fail on match ambiguity or CI layout drift. Reclassifies focus contract -> live in the Linux coverage manifest and updates the two pinned counts. The manifest gate is two-sided — a live claim must name a command the replay actually invokes — so the claim cannot drift from the file. |
||
|
Michał Pierzchała
|
1f8fdd0b5d |
fix: preserve Maestro clickable-first ordering (#1917)
* fix: preserve Maestro clickable-first ordering * test: cover Android Maestro clickable-first path * fix: keep Maestro fixture Android-only * fix: reveal Android Maestro targets in smoke scenario * fix: quote Maestro smoke assertion text * fix: retain Android Maestro clickability evidence |
||
|
Michał Pierzchała
|
5df5ec469d |
feat(maestro): add positional selectors (#1911)
* feat(maestro): add positional selectors * perf(maestro): resolve selectors once per snapshot |
||
|
Michał Pierzchała
|
0f05fa38a5 |
feat(maestro): add recursive tree selectors (#1910)
* feat(maestro): add recursive tree selectors * perf(maestro): resolve scroll selectors once |
||
|
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
|
40e4b0dd3e |
docs(agents): restore and enforce progressive disclosure (#1888)
* docs(agents): restore and enforce progressive disclosure * test(maestro): pin typed selector fallback signal * docs(agents): address progressive disclosure review * docs(agents): restore orphaned traps and close guidance-gate bypasses - AGENTS.md: skills carry a minimal start/routing card; command semantics stay in versioned CLI help (the skills contract enumerates two skills by hand, so prose retains ownership for the rest) - testing.md: restore the two local-only XCTest snags CI never hits (unsigned-bundle policy refusal signature + first-run automation permission) - scripts/gate/routing.ts: record GitHub's 300-changed-file path-filter limit at the paths-ignore assertion it bounds - agent-guidance-contract.test.ts: recurse docs/agents so nested guidance cannot evade the byte budgets while the gate stays green |
||
|
Michał Pierzchała
|
dd2a18ed4d |
perf(check-affected): stop running coverage locally, CI stays authoritative (#1908)
* perf(check-affected): stop running coverage locally, CI stays authoritative The `coverage` gate re-ran the affected Vitest suite under instrumentation on every `check:affected --run`, adding real overhead for signal the dedicated `Coverage` CI job already enforces on every PR. Mark it GitHub-authoritative and let `vitest-related`/`unit`/`provider-integration` run locally on their own instead of being folded into a coverage pass. Also removes the now-dead dedupe machinery in run.ts that existed only to support the local coverage-instrumented run. * fix: keep affected tests fast and bounded |
||
|
Michał Pierzchała
|
17bdca76cc |
refactor: migrate wait to request-bound runtime (#1875)
* refactor: migrate wait to request-bound runtime * fix: preserve native selector wait observation * fix: classify wait observations as conditional * refactor: compact conditional runtime declarations * fix: isolate selector runtime intents |
||
|
Michał Pierzchała
|
494eb52f66 |
refactor: migrate get to the request-bound device runtime (#1877)
* refactor: migrate get to the request-bound device runtime `get` declares `elementReadRuntimeUse` (required `captureSnapshot`, preferred `readTextAtPoint`), admits once from exact owner facts, refuses before binding, and binds exactly once. Its capability bucket, the static HarmonyOS/Web command sets that augmented it, and `requireCommandSupported` admission for `get` are gone; `'get'` leaves the `createSelectorRuntime` capability union. The neutral `readTextAtPoint` operation replaces the branch-per-family legacy `read` dispatch on the `get` path. Every local family and both providers now classify it exhaustively — Web, HarmonyOS, Vega and every provider row report it unavailable, which is behaviour-preserving because the legacy dispatch had no arm for them and threw on every call before falling back. R36 is the new parametrized cutover row. * fix(get): admit before the direct-iOS fast path; close the element-read outcome Review blockers on #1877. 1. `dispatchGetViaRuntime` could complete the direct-iOS selector query before `resolveBoundGetRuntime`. Once `get` declares `device-runtime`, ADR 0019 requires resolve -> admit -> bind before anything in the request path operates, so admission now runs first for every target shape and the fast path is a fast path *within* an admitted request. Regression: an eligible direct selector cannot operate when facts refuse admission. 2. `readTextAtPoint` returned `Promise<string>` and `readTextForNode` caught any throw and fell back, assigning a typed diagnostic after an untyped failure. It now returns a closed `ElementTextReadOutcome`; fallback happens only for the contract's classified reasons; unexpected errors propagate. The reason union is derived from its runtime list so the two cannot drift, and an unhandled reason is a compile error at the consumer. This retires the generic catch the start record promised. * feat(daemon): land the selector capture seam with get as its first consumer Takes ownership of the request-bound selector capture seam from #1876, which cannot ship standalone: with find's cutover deferred it had no consuming command (ADR 0019 §10) and was not dead-code clean (check:production-exports 19 -> 20). `get` is its first consumer, so it lands here. Adopts find's handoff as given. The one shape change, approved by the coordinator: the selector family gets its own capture uses carrying a PREFERRED `readTextAtPoint`, declared ALONGSIDE the snapshot uses so `snapshot`/`diff` keep binding exactly what they bind today. The read is surfaced through the existing arms of `bindSnapshotCaptureRuntime`, reusing the same selectActiveAppSnapshot / selectSnapshotWithoutActiveApp selectors — no second plan-to-operation dispatch. `get` now runs through `createBoundSelectorRuntime`; `resolveBoundGetRuntime` and its test are deleted as superseded, and `'get'` leaves the `createSelectorRuntime` capability union. The legacy read adapter survives for `find <q> get text` and is selected by which command constructed the runtime — never by failure, family, environment, or flag — so `get` cannot reach it. It retires in find's cutover, where the last consumer moves. * refactor: retire the read dispatch alias across both selector consumers Read-only `find` now constructs a BOUND selector backend, so `get text` and `find <q> get text` execute the same bound `readTextAtPoint` instead of one binding it and the other dispatching the legacy `read`. This moves find's READ LEG only: find's descriptor stays LEGACY_PLATFORM_EXECUTION and it claims no cutover row. With no consumer left, the whole chain goes: the `read` registry entry and its `dispatch: {}` projection, `DISPATCH_HANDLERS.read`, `handleReadCommand`, `interaction-read-legacy-dispatch.ts`, and the duplicate platform reader branches it carried. `read` was the only `dispatch-alias` descriptor, so that catalog group goes too. Deleting the registry entry drops 'read' from DescriptorDispatchCommandName, which makes a surviving DISPATCH_HANDLERS.read a compile error rather than something R36 has to police. R36 now claims the retirement it can prove. `find.test.ts` is over the size tripwire, so its handler invocation is extracted to find-handler-fixture.ts and the pin lowered 1237 -> 1221. * refactor(daemon): apply the seam addendum after #1876 was re-scoped Two edits, per find's ADDENDUM.md: 1. `includeRects` returns to `buildRuntimeCaptureInput`. It was removed from #1876 as unconsumed; the selector capture path is genuinely its first consumer (a Web rect capture requests bounds explicitly), so it lands here under the same rule that moved the seam. `snapshot`/`diff` pass nothing. 2. The per-capture `signal` is dropped, not restored. `CaptureSnapshotInput` has no such field on this stack — it moved to `wait` (#1875) with the regression that proves per-poll abort and quiescence. `get` captures once per resolution and never polls, so nothing here needs it. The seam test and fixture coverage for it moves with the contract rather than being kept against a field that no longer exists. * refactor(get): retire the direct-iOS selector shortcut `get` declares device-runtime, so its request path must reach the platform only through operations R36 declares. `dispatchDirectIosSelectorGet` reached `runAppleRunnerCommand` through a path the row declares no operation for; admitting before a bypass is not executing through the seam, so the bypass is removed rather than ordered after admission. Every target shape — including the simple iOS `id=` selector — now resolves through the bound capture. `queryDirectIosSelector` itself stays: `offscreen-target-probe.ts` still consumes it and it remains single-copy. `dispatchDirectIosSelectorIs` belongs to `is` (#1883). Two get-only helpers (`readDirectIosGetSelector`, `buildDirectIosGetResult`) became unreachable and are deleted with the caller. Declaring `querySelector` as a fact-admitted preferred operation was rejected on duplication, not correctness: the offscreen probe takes a plain session and cannot consume a bound operation, so it would ship the query twice until Wave 5 moves the probe — the deferred-duplication shape this PR was already overruled for on the `read` alias. It returns as a declared, §9-measured operation in a later unit that also moves the probe. Cost, stated plainly: `get text id=…` loses its tree-capture skip on iOS. No fallback was added and the latency is not recovered elsewhere. R36's singularExecution claim is now what the code does rather than aspirational. * refactor: ride the Interactor seam for the element read; drop the bespoke host Two operations of the same class were reaching their mechanics two different ways: `findText` rides `Interactor` via `localInteractors.resolve`, while `readTextAtPoint` had its own host port. That is duplication of MECHANISM, so the read now rides the same seam. `Interactor` gains `readTextAtPoint?`, implemented on the Apple, Android and Linux interactors where those mechanics already live. `src/platform-runtime-element-text-host.ts` and its `elementText` host wiring are deleted; the contract binds through the resolver exactly as the snapshot runtime does. Size honesty: this removes an 89-line module but the four readers still have to exist, so they moved into the interactors rather than vanishing. Net production change is ~4 lines, not ~89. The duplication of mechanism is what is actually fixed; Wave 5/6 retires the seam for both operations together. Also from the size investigation: - `ElementTextRuntimeExecution` was byte-identical to `SnapshotRuntimeExecution`; removed and reused, as `find-text-runtime.ts` does. - Removed a stranded, stale comment in `selector-capture-binding.ts` that still claimed a duplication this branch had already retired. - `FrozenUnavailablePlatformRuntimeFacts` is derived from its input type rather than restated, removing a 14-line clone group my new cell had pushed over the detector threshold. * refactor: migrate is to the request-bound device runtime (#1883) * refactor: migrate is to the request-bound device runtime `is` declares the shared selector capture use, admits once from exact owner facts, refuses before binding, and binds exactly once. Its capability bucket, the static HarmonyOS/Web command sets that augmented it, and `requireCommandSupported` admission for `is` are gone; `'is'` leaves the `createSelectorRuntime` capability union. Admission now runs BEFORE the direct-iOS selector fast path. ADR 0019 requires resolve -> admit -> bind before anything in a `device-runtime` command's request path reaches the device, so that query becomes a fast path *within* an admitted request rather than a way around exact-owner facts. The rule is documented once, on `createBoundSelectorRuntime`, replacing the two duplicated call-site comments `get` and `is` were each carrying. Declared behaviour change: `is` takes the active-app plan split, so the facts decide per family. On iOS `appBundleId` is the XCUITest attach target — with no tracked app the runner's own process comes to the foreground, displaces the app under test, and the capture then answers confidently about the runner's own blank screen. An iOS `is` on a session with no tracked app is now a typed SESSION_NOT_FOUND refusal carrying the `open` hint. Refusing beats displacing-and-lying. Android captures the real launcher in that state and is unchanged, which is what the platform facts already encoded. The two Apple watchOS cells move from capability-admitted-then-runner-failure to a typed unavailable refusal, the same classification snapshot, diff, and get already landed. R37 is the new parametrized cutover row. `find` keeps `createSelectorRuntime` and its `requireCommandSupported` call, so `captureData` stays optional and `captureSnapshotWithInteractor` stays: this unit is not the last selector unit. * fix(is): a failing iOS assertion fails instead of exiting zero Reverses part of #557, on thymikee's explicit instruction. `is` is an assertion: the docs state it "exits non-zero on failure". The direct-iOS fast path broke that contract — it reported a failed predicate as a completed command, so on device $ agent-device is text id=… "Wrong Expected Text" Passed: is text (exit 0) because `{ok: true, pass: false}` reaches `isCliOutput`, which renders "Passed: is <predicate>" without reading `pass`. A failing assertion reported as success lets a replay run on past a broken state. Now: Error (COMMAND_FAILED): is text failed for selector id=…: expected="Wrong Expected Text" actual="Apple Account, …" (exit 1) The renderer needed no patch: a negative can no longer produce a success envelope, so it is correct by construction. Direction chosen deliberately. Making the two paths agree could have gone either way, and "an agent asked a question and got an answer" is a real argument for the other one. This follows the DOCUMENTED contract rather than merely the incumbent behaviour, and the alternative is a far larger change: a zero-exit `is` would alter every platform and path, break scripts that rely on it failing the shell, and needs its own PR, docs, and probably a major version. It is also already how `is hidden` and `is exists` behave end to end. PASSING assertion, and that arm still answers with zero captures (pinned). Only the negative falls through — what #557's own summary asked for, "preserving snapshot fallback for misses", refusing fallback only for hard failures like ambiguity. The fall-through was #557's own design, never armed: the `| null` return and the caller's `if (!payload) return null;` guard were unreachable. This makes that dead guard live. Measured on iPhone 17 (median of 9, warm daemon): predicate holds 0.14s / 0 snapshots, unchanged; predicate fails 0.25s / 1 snapshot. ~+0.11s on failing assertions only. Correctness gain beyond the envelope: the fast path evaluates a ONE-NODE tree, so `visible` cannot see the ancestor geometry a list row inherits and its negative can be wrong. Falling through re-asks the real tree and can turn a spurious negative into a pass. The #557 pin moved with its reasoning at the pin site. * fix(layering): let a cutover row state a data-only admission retirement Review blocker on #1883: R37 claimed `legacyRetirement.routeNames: ['WEB_QUERY_COMMANDS_WITH_IS', 'HARMONYOS_IS_SUPPORT']`. Neither identifier has ever existed. They satisfied the non-empty shape check while proving nothing — the vacuous registry claim AGENTS.md warns about, and a green gate that would stay green if the deletion were reverted. The cause was the model, not the row. Every `LegacyRetirementClaim` form names something that must NOT exist, which a row can always satisfy by inventing a name. `is` retired no module, route, or dispatch projection because it had none: its legacy admission was a capability bucket plus membership in two static platform command sets, so its real retirement is a DATA deletion the model could not express. Rather than patch around that with sentinels or a per-command policy file — both forbidden by the playbook — this generalizes the model. `staticCommandSets` names the sets themselves and is proven from both sides: each must still be DECLARED in production source, and must no longer list the command. A fictional set fails the first half; a skipped deletion fails the second. That is what an identifier-shaped claim cannot state. R37 now claims HARMONYOS_SUPPORTED_COMMANDS and WEB_QUERY_COMMANDS, which is the deletion it actually performed. Planted red, both halves, against the real gate: [R37 is-runtime-cutover] 2 violation(s): (is cutover row):1 — claims retired static command set 'WEB_QUERY_COMMANDS_WITH_IS', which no production source declares (is cutover row):1 — claims retired static command set 'HARMONYOS_IS_SUPPORT', which no production source declares [R37 is-runtime-cutover] 2 violation(s): src/core/capabilities.ts:59 — static command set WEB_QUERY_COMMANDS still admits is so the exact claim that shipped is now rejected by name, and so is restoring the membership it claims to have removed. Mechanism cases live with the other planted-row tests; layering goes 177 -> 181. * test(is): pin the exit-code guarantee independently of what answers the predicate Prep for the Blocker 1 retirement, which deletes `buildDirectIosIsResult` — the function the #557 reversal fixed. The reversal's guarantee must not evaporate with it, so it gets a case that does not know how the daemon decided. `is` is documented to "exit non-zero on failure". The reversal proved that at the JSON envelope; nothing pinned it at the CLI boundary, which is where the defect was actually visible (`Passed: is text`, exit 0). This asserts the CLI contract directly: a `predicate_failed` response exits 1 and never renders as passed. It survives the retirement untouched, because it asserts the outcome rather than the path. Planted red with the exact pre-#1739 envelope the shortcut produced (`{ok: true, data: {pass: false}}`): `exitSpy.calls` is `[]` — no exit call at all — so the case fails, which is the regression it exists to catch. Unpushed on purpose: the restack will carry it into the retirement cycle. * refactor(is): retire the direct-iOS selector shortcut thymikee's ruling (option b). `is` declares `device-runtime`, so its request path must reach the device only through the operations R37 declares. It did not: a simple iOS `id=`/`label=` target was answered by a direct XCUITest querySelector without any capture, ordered after admission but not executing through the seam. This is not retired because it was wrong. `wait` hypothesized that the degenerate one-node evaluation mis-answers `is visible` for off-viewport nodes, traced it through the code convincingly, then tested it on device and it did not reproduce — XCUITest's own query is conservative about visibility, so the degenerate evaluation never gets the chance. It is retired because it was an undeclared, unmeasured bypass that made R37's singularExecution claim false: the same class of untruth as the sentinel retirement names fixed in the previous commit. Declaring querySelector as a real operation instead was rejected for a concrete reason: offscreen-target-probe.ts consumes queryDirectIosSelector with a plain session and cannot take a bound operation, so declaring it now would ship it twice until Wave 5 moves the probe — the deferred-duplication shape that got get's read deferral overruled. It returns as a declared, fact-admitted, section 9-measured operation in the unit that also moves the probe. Retired: dispatchDirectIosSelectorIs, its call site, buildDirectIosIsResult, and resolveDirectIosSelectorQuery — each had exactly one caller, all on this path — plus the ResolvedDirectIosSelectorQuery type they orphaned and two imports. queryDirectIosSelector itself stays: the offscreen probe still consumes it and it remains single-copy. Latency cost, stated plainly and not softened: a held predicate on a simple iOS selector goes from ~0.14s with no capture to ~0.25s with one, measured as the median of 9 warm runs on iPhone 17. There is no fallback and no fast path. R37's comment finally describes the code: "every predicate answers from the resolved tree" was written while the shortcut existed. Its scope is now stated too, so it is not read as absolute — the Android foreground-blocker diagnostic still reaches adb on the failure path, where it cannot produce or change a verdict; that edge is pre-existing, co-owned with wait, and recorded as Wave 6 denominator work with R22's appState as its declared replacement. Seven tests lost their subject. Those whose only content was the shortcut's own mechanics are deleted; the outcome-level ones are retargeted and keep asserting what survives. --------- Co-authored-by: agent <agent@local> * fix(contracts): a falsely advertised element read fails as a contract bug An owner whose facts advertised `readTextAtPoint` but whose interactor cannot perform it was reported as `{ status: 'unreadable', reason: 'surface-not-readable' }`. That put a contract violation inside the closed reason set that licenses falling back to the captured tree, so `get text` answered from potentially stale snapshot text precisely because the runtime lied about itself. ADR 0019 §2 requires the mismatch to fail as `runtime-contract-invalid`; it now throws. Removing the only producer of `surface-not-readable` made that reason dead: no path can reach it, since an interactor that HAS the read maps a blank or absent answer to `no-text-at-point` via `elementTextRead`. Dropped from the union, its consumer switch arm, and both test lists. `classifiedFallbackReason`'s `never` arm stays — it is what makes adding a reason a compile error rather than a silent untyped fallback. Deduplication found while auditing the change: - `invalidRuntimeContract` was module-private in `platform-runtime.ts`. It now owns its own module so both runtime modules share one construction. It is deliberately not exported through the platform facade: that facade must stay exhaustive over its sources, which would make this a public symbol with no external consumer. - The 8-field runner execution projection was written out three times (`snapshot-runtime-capture-input.ts`, `interaction-read.ts`, `screenshot-runtime.ts`). One `runtimeExecutionFromContext` now serves all three; `screenshotExecutionFromContext` keeps its name and delegates, since `ScreenshotRuntimeExecution` and `SnapshotRuntimeExecution` are the same type. Dropping a field here silently strips request id, log/trace paths, XCUITest overrides, or runner lease context — an operation that still answers but runs unconfigured, which is exactly the defect the wait unit hit as a P1. Red before green: with the old guard restored the new regression fails with "Missing expected rejection" — the call resolves instead of throwing, which is the silent degradation it exists to forbid. --------- Co-authored-by: agent <agent@local> |
||
|
Michał Pierzchała
|
250e30a578 |
test(bench): falsification fixtures for oracles + typed runner outcomes (#1893)
* test(bench): falsification fixtures for oracles + typed runner outcomes
Two deterministic PR-time quality gates for the help-conformance bench
(the repo's single non-gating small-model planning oracle):
- Every EXPECTATION_SCORERS entry in help-conformance-case-checks.mjs
now has a falsification fixture (a minimal passing witness plus at
least one known-bad counterexample, and a metamorphic variant where
useful) in the new help-conformance-expectation-fixtures.ts, run
through the real validatePlanCommands/scoreExpectations pipeline.
help-conformance-expectation-falsification.test.ts is the "what
enumerates N" completeness gate: a new named expectation with no
fixture fails it. Counterexamples cover swallowed lifecycle command
prefixes, unsupported flags/selectors, pseudo refs, shell operators,
and invalid positional ordering.
- help-conformance-runner-output.mjs now returns a discriminated
RunnerOutcome ({kind:'success',commands}|{kind:'runner-error',
message,reason}) instead of a raw-string success inference. Only a
'success' outcome ever reaches validatePlanCommands/scoreExpectations
in runCase, so a runner-error result can no longer also carry
model-validation checks, and an all-runner-error aggregate now
reports passRate: null (rendered as "N/A") instead of "0/0 (0%)".
Fixes #1481
* refactor(bench): dedupe RunnerOutcome construction, drop leftover narrowing
Thermo-nuclear pass over 4b2df0a38's diff:
- help-conformance-bench.mjs's runOutcome() catch block was hand-building
the exact {kind:'runner-error', raw, message, reason} shape that
runner-output.mjs's private runnerError() helper already constructs for
its own two error paths. Export it as runnerErrorOutcome so the
discriminated union has exactly one constructor for its error variant,
reused by both error sources instead of duplicated.
- runCase's two return branches repeated the same
{runner, caseId, trial, outputPath} fields; pulled into a shared `base`
object.
- Reverted bench.test.ts's rateLimitedOutcome block: it had an explicit
`: RunnerOutcome` annotation and an if/throw narrowing guard, added only
to give fallow's dead-code checker a "real consumer" of the type before
the actual fix (adding the .d.mts to .fallowrc.json's ignorePatterns,
matching the existing sample-outputs.d.mts precedent) was found. That
workaround is now unnecessary scaffolding — replaced with the same
flat assert.deepEqual style the surrounding assertions already use.
|
||
|
Michał Pierzchała
|
2d7a310dd0 |
fix(maestro): restore conformance invariants (#1889)
* fix(maestro): restore conformance invariants * fix(maestro): use exact iOS presentation mappings |
||
|
Michał Pierzchała
|
80b4769230 |
test(fuzz): structured CLI/Maestro generators that reach command validation and assert error codes (#1781 B2) (#1866)
* test(fuzz): structured CLI/Maestro generators that reach command validation and assert error codes (#1781 B2) * test(fuzz): pin the rediscovered #1433 excess-positional case and keep numeric flag samples inside their range * style: apply oxfmt to the new fuzz modules * perf(fuzz): derive the CLI validation surface lazily so unrelated harness paths keep their startup * test(fuzz): resolve validation generators in the run path so corpus replay keeps its small module graph * test(fuzz): weight the CLI budget toward command validation, pin the finite classes as seeds, guard lazy surface derivation * docs(testing): describe the validation lane's layer split, seed-pinned classes, and PR-time gates * refactor(fuzz): split the validation generator into CLI and Maestro modules, mirrored in tests * refactor(fuzz): collapse the flag-shaped mutation classes and seed literals, derive class coverage from declarations * fix(fuzz): hash every case-generation module in configHash, guarded by an import-closure test * test(fuzz): assert CLI command and flag-key coverage against the registry, and close the six gaps it found |
||
|
Michał Pierzchała
|
393eb30a28 |
ci: give check:affected real Apple ownership rules and route ios.yml on them (#1781 A9-2) (#1857)
* ci: give check:affected real Apple ownership rules and route ios.yml on them (#1781 A9-2) Device-lane ownership by platform family in the affected selector (scripts/check-affected/device-lanes.ts): a TypeScript-only Apple change now carries replay-ios/replay-ios-device/replay-macos in a narrow plan, other families own only their own lanes, shared runtime surface owns every lane, unit tests own none. Golden tables (contracts/fixtures) own the parity unit test and both runner builds instead of failing open. ios.yml pull_request paths-ignore is routed on that ownership; the gate manifest asserts the list against the selector over every tracked path both ways (scripts/gate/routing.ts, ROUTED_LANES). push to main is unfiltered. Path coverage exempts declared manual-only checks the way owned does. * ci: tighten routing assertion shape (fallow: unused exports, complexity) * ci: name parked checks in check:affected --run skips * ci: bound the routed-lane exemption to sibling workflows (review of #1857) The exact-name .github exemption was unbounded: naming the lane's own setup-apple-runner-build or boot-ios-test-simulator action skipped the lane that runs them and the manifest stayed green. Lane now carries the transitive composite-action closure plus its own workflow file (Lane.uses, same walk declaredGates does), and the exemption refuses anything in it. Also: an unowned path under an ignored root (a non-TS fixture under a family root) asked for the ignore entry to be removed, which would un-route every sibling in that tree; it now asks for a selector owner. Both cases pinned, both proven red against the pre-fix code. Documents GitHub's 300-changed-file path-filter limit in docs/agents/testing.md. * ci: close the routed-lane exemption over composite-action support files Lane.uses recorded only each composite action's action.yml, so a support file the descriptor executes was exemptible as if it were an unrelated sibling workflow: ios.yml uses setup-fixture-app, whose action.yml runs "$GITHUB_ACTION_PATH/fetch-artifact.sh", and that script runs its siblings resolve-artifact-name.sh and trusted-artifact.mjs — references that exist only inside shell, one level past anything YAML parsing sees. The closure unit is the action's directory now. It needs no shell model and cannot miss a file however deep the reference chain runs; the coarseness is harmless because a file in an action's own directory belongs to that action. All three files pinned, red against the descriptor-only closure. |
||
|
Michał Pierzchała
|
735ab7672a |
refactor(daemon): one capture-input builder and one admit-then-bind step (#1876)
Behaviour-neutral. No descriptor changes platform execution, the cutover table is untouched, and no contract surface is added. - buildRuntimeCaptureInput moves to its own module so every request-bound capture consumer builds CaptureSnapshotInput one way. - The admit-then-bind sequence in the snapshot/diff resolver becomes one named step, ready for the selector units' second caller. - handlers/find.ts splits into focused target-capture and match-resolution concepts (600 -> 346 lines); behaviour unchanged. Co-authored-by: agent <agent@local> |
||
|
Michał Pierzchała
|
d8e03aea9b |
refactor: migrate screenshot to request-bound runtime (#1878)
Retires the last dispatchCommand edges for screen capture: the generic-route command, the sparse-snapshot fallback, and the Android snapshot-timeout evidence capture all admit exact owner facts and bind once (ADR 0019, cutover rule R39). --overlay-refs becomes part of the declared use, so a target that can capture pixels but not a tree is refused before anything is written to disk. |
||
|
Michał Pierzchała
|
d07b837621 |
test: classify the runner XCTests — pure decisions to a macOS host lane, simulator semantics gated os(iOS) (#1781 A7) (#1861)
Every declared AgentDeviceRunnerUITests method now belongs to a lane, and the #if guard is the classification: AGENT_DEVICE_RUNNER_UNIT_TESTS alone means a pure runner decision (runs on the macOS host on every PR — ci.yml's existing compile job now executes the bundle it builds), '&& os(iOS)' means runner/XCTest semantics (simulator lanes only). check:xctest-selection evaluates the guards per platform, derives each lane's reach, and fails on a flagged identifier that is undeclared or uncompiled on that lane, on a declared test no lane reaches (found the two tvOS-only tests, dark since birth — widened to os(tvOS) || os(macOS)), and on testCommand reaching any lane. The host and nightly lanes assert executed == derived reach, so a missing -D flag or a guard that compiles a file out reads red, not as a smaller green. One duplicate test deleted (sparse-verdict assertions folded into its twin). |
||
|
Michał Pierzchała
|
99754dc69c |
fix(layering): resolve relative imports inside workspace packages referencing #1781 (#1872)
* fix(layering): resolve relative imports inside workspace packages resolveTargetFile() dropped any relative import whose resolved path didn't start with src/. Since #1490 W0 added packages/*/src/** to the source set, every intra-package relative import (e.g. a facade re-exporting a sibling file) was silently invisible to the layering graph — R4 value-cycle rejection and depgraph reverse reachability both stopped at the package facade. Resolve relative specifiers that land under packages/<name>/src/ too, while still refusing anything outside src/ and packages/*/src/. Verified against the real tree: 448 previously-invisible value edges and 436 type-only edges now resolve, but none of them close a new R4 value cycle or grow the R9 type-cycle SCC, so the R6/R9 baselines are unchanged. Refs #1781 * style: apply oxfmt to regression test |
||
|
Michał Pierzchała
|
6984a1e095 |
fix(layering): list the whole zone when R10's type-cycle ceiling is exceeded (#1852)
* fix(layering): list the whole zone when R10's type-cycle ceiling is exceeded The per-zone R10 violation named members.find(<zone match>) — the alphabetically-first zone member, a file that had been in the cycle all along — so the +1 in #1825 x #1779 was found only by diffing largestTypeCycleMembers between commits. The ceiling records a count, not a membership, so the gate cannot name the joining file; it now lists every member of the over-budget zone and annotates the ceiling table instead. Closes #1837 * fix(layering): state the zone overflow in net terms Review nit: the overflow is net growth over the ceiling, not a join count (two joins and one departure print "1"), so the message no longer claims N members joined. |
||
|
Michał Pierzchała
|
f3d5b3d92c |
refactor(daemon): admit-before-bind as an admitted-plan token; retire the R32 syntax policy (#1841)
* refactor(daemon): admit-before-bind as an identity-keyed admitted-plan token; retire the R32 syntax policy admitRuntimePlan (was inspectRequiredRuntimeUse) takes the plan and, on success, mints an AdmittedRuntimePlan: a nominal class instance with nothing readable on it. Its payload — a frozen copy of the device the facts were read for, and the plan — lives in a module-private WeakMap keyed by the token's exact identity, and the only way to read it is unwrapAdmittedRuntimePlan, which refuses anything not minted here. The snapshot owning interface (resolveBoundSnapshotCaptureRuntime, #1847) admits through it and its private binder takes only the token: no bare plan, no separate device, and no look-alike — a spread lacks the #private member (not assignable), a Proxy around a real token types as the token but is a different identity (refused at unwrap), Object.assign/defineProperty throw on the frozen instance, and the class value is not exported so its constructor is not nameable. That retires scripts/layering/runtime-command-cutover-snapshot.ts — R32's per-command AST policy (call-shape recognition of the admission and a text sniff for a local admission) — and the source-regex test beside the descriptor tests. The generic row keeps retirement, narrowing, and singular execution; the manufactured-proof column now also rejects casts to AdmittedRuntimePlan. Planted reds: token degraded to a plain public shape → 2 unused @ts-expect-error directives; unwrap reading the token surface via getters → the Proxy regression fails; getter-based branded literal → the runtime retarget test fails. * docs(agents): the ADR 0019 unit checklist teaches the shipped admission API #1836 documented inspectRequiredRuntimeUse with a forward note pointing here; this PR makes admitRuntimePlan real, so the row now teaches it plus the identity-keyed unwrap the binder uses, and points at the shared snapshot/diff owning interface as the model. |
||
|
Michał Pierzchała
|
37b1bc8cbd |
refactor: migrate viewport to request runtime (#1864)
* refactor: migrate viewport to request runtime * fix: preserve viewport cutover evidence |
||
|
Michał Pierzchała
|
3f0f706f0b | refactor: migrate diff to request-bound runtime (#1847) | ||
|
Michał Pierzchała
|
f03c0309a1 |
fix: derive iOS transition snapshots from visible presentation (#1831)
* fix: project iOS transition semantics * fix: derive iOS transition semantics from visible state * fix: preserve iOS presentation context for scoped snapshots * fix: confirm broad iOS transition settlement * ci: run coordinate input regression on pull requests * test: mock migrated snapshot capture seam * fix: confirm transitions across snapshot backends * fix: arm transition confirmation after first capture * fix: settle against immutable action baseline |
||
|
Michał Pierzchała
|
6a8beb653e |
feat(mcp): compact server instructions in both eras + MCP-only help tool (#1839)
* feat(mcp): compact server instructions in both eras + MCP-only help tool (#1833) MCP-only clients got no workflow guidance: server/discover carried two sentences, legacy initialize carried nothing, and the CLI guides (agent-device --help, help <topic>) were unreachable over MCP. - MCP_SERVER_INSTRUCTIONS: one MCP-phrased workflow card (<2 KB, the Claude Code truncation limit) returned by server/discover and legacy initialize alike. - help tool, router-owned (not a command descriptor): no topic -> the CLI decision card; topic -> agent-device help <topic|command> text, prefixed with the one-line CLI->tool-property mapping; unknown topic -> isError listing the topics. listCommandTools() stays descriptor-only for the AI SDK; the router composes descriptors + help. - Move src/cli/parser/cli-help{,-overview}.ts to src/cli-schema/ so src/mcp (rank 3) can import the renderers without a layering back-edge into src/cli (rank 6). * fix(mcp): name terminal-only commands in help guides; colocate cli-help tests with their sources - The MCP guide preamble claimed every `agent-device <command>` line is a tool of that name; `help web` tells the reader to run `web setup` / `web doctor` and no `web` tool exists. The preamble now lists the exact CLI-only set (listCliCommandNames minus listMcpExposedCommandNames) — derived, not scanned out of prose where `device`/`web` are ordinary words. Regression: help web names `web` as terminal-only, and the listed set equals the registry difference. - cli-help-*.test.ts move from src/cli/parser/__tests__ to src/cli-schema/ to mirror the moved sources. * perf(mcp): tighten the guide card, tool description, and preamble Instructions card 1572 -> 1378 bytes (paid every session), tool description and preamble trimmed, HELP_TOOL built once as a const. Bundle delta vs main 3189 -> 2715 bytes; the remainder is the guide text itself, which the bundle carried in no MCP-phrased form before. |
||
|
Michał Pierzchała
|
0fb38f1da2 |
test: prune abandoned test-run tmp directories at run setup (#1834)
* test: prune abandoned test-run tmp directories at run setup A run killed before its teardown (tool-timeout SIGKILL, OOM, cancelled job) left /tmp/agent-device-test-run-<pid>-* behind, and check:tmpdir-leaks — which runs after test:unit in check:unit — flagged every dead-pid directory it found. It could not tell this run's leak from a historical one, so one killed run made every later, otherwise-green gate on the host fail. Both TMPDIR redirection entry points (the Vitest global setup and the node --test wrapper) now prune dead-pid run directories before creating their own, printing one [tmpdir] line when they did; the post-run check keeps its semantics and can now only ever name the run that just finished. Live owners (a concurrent run in another worktree) are never touched. The root/prefix constants move into check-tmpdir-leaks-model.ts, next to the liveness classification, so the setup can import the prune without a cycle. * test(tmpdir): a run directory is live while any process still holds it as TMPDIR, not only while its owner runs Review (P1): owner-pid liveness alone would prune a directory out from under the orphaned children of a SIGKILLed run — the node --test chain, Vitest forks, or a daemon a test spawned all keep running with that TMPDIR. The liveness model now reads every process's TMPDIR (ps -E on macOS, /proc/<pid>/environ on Linux) and treats a run directory as live while its owner pid is alive OR any process's TMPDIR points into it; both the prune and the post-run leak check use it. Regression: a wrapped probe spawns a detached long-lived child, only the wrapper is SIGKILLed, the next prune preserves the directory; after every consumer exits, the next prune removes it. Planted red with owner-only liveness: the orphaned directory is pruned. |
||
|
Michał Pierzchała
|
423927fdd8 |
chore(mutation): shrink to report-only — drop the ratchet, baseline and graduation (#1457, #1781) (#1828)
* chore(mutation): shrink the lane to report-only (#1457, #1781 wave 2) The mutation harness's two real catches (#1474, #1475) both came from humans reading the weekly score report. The ratchet half never operated: the baseline was committed exactly twice ( |
||
|
Michał Pierzchała
|
d76e0f94e9 |
refactor: migrate snapshot to device runtime (#1779)
* refactor: migrate snapshot to device runtime * refactor: complete snapshot runtime policy cutover * test: enforce snapshot owner-facts admission * refactor: consolidate desktop snapshot capture * fix: scroll to visible iOS smoke targets * fix: close snapshot cutover alias bypasses * fix: constrain snapshot admission identity flow * fix: enforce snapshot admission through owner facts * fix: adapt replay source tests to snapshot runtime |
||
|
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
|
4b44c1c53a |
chore(test): remove the contention retry and shrink the subprocess-stub project (#1781 A4) (#1827)
The enumerated single-retry policy (#1419) has fired zero times since it landed on 2026-07-29: 0 of 234 sampled Coverage-job lane envelopes (2026-08-11 to 2026-08-18) have retryCount > 0, and none of 17 recent failed runs was retried (5 refused "outside the enumerated retry list", 4 refused "unhandled error"). All three trackers its entries pointed at (#1098, #1414, #1419) are closed. It cost ~1,454 LOC, a per-run secret marker threaded through a setup file on every Vitest project, and a standing obligation for every future gate reporter to call the blocker bus. Delete the scripts, tests and fixtures, the check:contention-retry script and gate, the envelope artifact upload, and the runner-timeout setup file; test:coverage:ci is a plain `vitest run --coverage` again. lane-envelope.ts stays: the mutation, fuzz and concurrency-torture lanes build their envelopes from it. run-blocker-bus.ts goes: its only consumer was the retry's failure sink, and its only publisher already fails the run by setting process.exitCode. Keep the subprocess-stub project for the three files that really spawn (client-metro, fuzz harness, fuzz corpus-replay) and drop the three that run in 31/212/277ms in CI, which cannot contend for anything. The list is now a plain array in vitest.config.ts with the reason at each entry. Membership and the project's kill criterion live in #1823. Because test:coverage:ci is a bare vitest run, the gate manifest reads its projects directly, so OPAQUE_RUNNERS no longer needs it and an unrun Vitest project becomes unrepresentable rather than detected; the audit test now constructs that state by project-scoping the script. |
||
|
Michał Pierzchała
|
142d156338 |
ci(ios): run the full XCTest suite nightly and check the PR test list (#1781 A7) (#1789)
* ci(ios): run the full XCTest suite nightly and check the PR test list (#1781 A7) * fix(ci): skip the runner server entry point in the nightly and validate both test flags * docs(ci): restate the nightly lane cost and timeout honestly * docs(ci): stop quoting XCTest counts that drift between commits * ci(ios): tighten the nightly timeout to the measured suite duration |
||
|
Michał Pierzchała
|
801734d433 |
feat(ai-sdk): add agent-device/ai-sdk tool set and document the MCP zero-code path (#1804)
* feat(ai-sdk): add agent-device/ai-sdk tool set and document the MCP zero-code path
Adds `createAgentDeviceTools()` under a new `agent-device/ai-sdk` subpath,
built from the same command registry the MCP server uses so both stay in
lockstep without a hand-maintained tool list. Introduces a `frameworkTier`
descriptor facet ('core' | 'extended') so the factory can default to a
curated perceive/act loop instead of handing a model dozens of tools.
`ai` is wired as an optional peer dependency, imported lazily inside the
factory rather than at module scope, so importing the subpath itself never
requires `ai` to be installed - only calling it does. The package's own
publishing gate (scripts/lib/shipped-imports.ts) is extended to recognize
peerDependencies as a valid resolution source, since this is the first
optional peer this package has shipped.
Also restructures the AI SDK doc around three tiers (zero-code via
@ai-sdk/mcp, the new typed tool set, hand-written tools) and fixes a stale
`needsApproval` reference in favor of the current `toolApproval` API.
* fix(layering): classify src/ai-sdk as a rank-4 zone
The layering guard requires every src/<folder>/ to be explicitly ranked or
unranked; the new src/ai-sdk/ subpath (added in the prior commit) was left
unclassified, failing CI's Layering Guard job. It sits at the same tier as
client/compat/daemon-server/metro/remote/sdk - a public integration surface
consuming mcp (3) and core (2), imported by nothing else in the tree.
* fix(ci): cover, exempt, and pack the new ai-sdk subpath
Fixes the remaining CI failures on the ai-sdk subpath commit:
- Coverage: src/ai-sdk/index.ts had no dedicated unit test (only manual/
integration verification), so changed-line coverage sat at 6.9% against
the 70% gate. Adds src/ai-sdk/__tests__/index.test.ts (core vs 'all' tool
filtering, session/platform pinning and schema hiding, error
normalization, toolApproval passthrough) with createCommandToolExecutor
and createAgentDeviceClient mocked the same way command-tools.test.ts
does, plus a dedicated missing-peer-dependency.test.ts that mocks `ai`
itself to throw, isolated to its own file so it doesn't affect the other
tests' use of the real, installed `ai` package. Changed-line coverage is
now 29/29 (100%).
- Fallow Code Quality: src/ai-sdk/index.ts and examples/sdk/ai-sdk-tools.ts
are entry points with no in-repo importer (reached only via package.json
exports / run directly), and the new subpath's exports are unused
internally by design - both need the same treatment src/sdk/*.ts and its
examples already have in .fallowrc.json.
- Integration Tests: test/integration/installed-package-metro.test.ts and
src/__tests__/package-exports.test.ts each hand-list every published
subpath and smoke-check it from a real packed install; added ./ai-sdk to
both so the new subpath is actually exercised, not just silently passing.
* fix(ai-sdk): hide MCP transport/config fields from the model too
createAgentDeviceTools() only removed session and mcpOutputFormat from tool
schemas. stateDir was still model-visible and reached the shared executor
as client configuration, letting a tool call redirect into a different
daemon state directory - defeating the "one pinned session" guarantee the
factory exists to provide. includeCost and responseLevel are MCP
tool-config knobs in the same category, irrelevant to this adapter.
Widens the hidden-field set to session/stateDir/mcpOutputFormat/
includeCost/responseLevel, and now strips them from the runtime input
inside execute() too (not just the schema), so the guarantee holds even if
a caller bypasses schema validation. The schema-properties filter and the
input filter now share one omitHidden() helper instead of two near-
duplicate implementations.
Addresses the P1 review comment on #1804.
|
||
|
Michał Pierzchała
|
04613ae8d3 |
ci: keep Bundle Size job green on transient GitHub comment failures (#1795)
* ci: keep Bundle Size job green on transient GitHub comment failures The size measurement and job summary had already succeeded on PR #1789 (run 32050847506) when the PR comment write got a 503 during a GitHub incident and failed the whole lane. --post-comment now retries 5xx / 429 / network errors (4 attempts, 1s/2s/4s backoff) on both the list and write calls. If it still fails, it prints a ::warning::, appends a note to $GITHUB_STEP_SUMMARY, and exits 0. Other 4xx (bad token, missing permissions) stay fatal. * refactor: split GitHub response classification to satisfy fallow complexity gate * fix: reconcile uncertain comment creates instead of re-POSTing; add regressions Retry now wraps the whole list -> write cycle rather than each request, so a create whose response was lost (network error / 5xx) is re-listed on the next attempt and turned into a PATCH of the marker comment instead of a duplicate POST. Splits the retry/classify helpers under the fallow complexity gate. Adds scripts/__tests__/size-report-post-comment.test.ts (unit-core): spawns the real script against a stubbed fetch and pins uncertain-create reconciliation, transient exhaustion (warn + exit 0), and fatal 4xx (nonzero, no retry). SIZE_REPORT_RETRY_BASE_MS lets the tests skip real backoff. |
||
|
Michał Pierzchała
|
ccf64f6797 |
ci: move parked device replay suites to a dispatch-only workflow (#1781 A1) (#1794)
* ci: move parked device replay suites to a dispatch-only workflow (#1781 A1) Both full-tier device jobs have failed every scheduled run since 2026-07-24: the Android suite inside full-tier scenarios that had never executed end to end, the iOS suite on varying steps. They move to .github/workflows/replays-manual.yml, which has no `schedule:`, so the schedule stops emitting a guaranteed failure while the suites stay runnable on demand. A job-level `if: github.event_name == 'workflow_dispatch'` would have looked the same and lied: `workflowLanes()` decides `qualifying` per workflow FILE and never reads job-level `if:`, so the manifest kept reporting replay-android, replay-ios, and replay-ios-device as scheduled-lane owners — the silent-owner-loss failure the manifest exists to catch. A separate file is what the file-level model already reads correctly. Those three checks now have no pull_request/schedule owner, so they are declared as MANUAL_ONLY_OWNERS rather than folded into UNPROVABLE_OWNERS, whose claim ("it runs, this loader cannot see it") is no longer true for replay-android. check:gate-manifest drops from 48 to 46 wired checks and names the three on every run. Two tests pin it: a dispatch-only lane is non-qualifying however many gates it declares, and every manual-only declaration must name a registered check that no qualifying lane owns, so a re-scheduled lane cannot keep a stale exemption. * ci: attest manual-only checks against their dispatch lane (#1781 A1) Review P1: MANUAL_ONLY_OWNERS was a negative allowlist — it proved each entry named a registered check no qualifying lane owned, but nothing tied the entry to a lane that can still run it. Deleting a parked job, or its run-gate step, would have left the manifest green and still printing the check as manual-only: parked coverage silently becoming deleted coverage. Each entry now names its dispatch lane, and a new 'manual-only' audit assertion resolves that name against the derived model: the lane must exist, must still be dispatch-only, and must still declare the gate. replay-android carries an explicit `opaque` flag because its gate sits inside the third-party emulator action's `script:` (#1429), so the job's existence is the whole attestation the model can make — and the flag says so rather than letting an unreadable lane look like a declaring one. Four regressions pin both directions: deleting a declaration reports the check as unowned; deleting the parked job fails with 'no workflow defines'; re-scheduling the lane fails until the entry is dropped; and a parked lane that loses its run-gate step fails unless the entry is opaque. * ci: make manual-only mean dispatch-only, not merely non-qualifying (#1781 A1) Review follow-up: the attestation checked `qualifying === false`, which is true of any lane that is not pull_request/schedule. Swapping `workflow_dispatch` for `push` in replays-manual.yml would have kept the audit green and the checks printed as manual-only, while the runs nobody starts by hand quietly started themselves on every push. The lane model now keeps the trigger names instead of collapsing them into that one bit, and the manual-only assertion requires `workflow_dispatch` and nothing else. Three planted regressions cover the gap the review named: a parked lane re-triggered by `push` fails, a parked lane with no trigger at all fails, and the loader test pins that trigger kinds survive into the model (a push lane reads `[push]`, the nightly reads `[schedule, workflow_dispatch]`). |
||
|
Michał Pierzchała
|
d8a7d03faf |
refactor: route application lifecycle through runtime facts (#1759)
* refactor: route application lifecycle through runtime facts Moves the canonical `open`, `prepare`, `close` and internal `runtime` descriptors behind package-owned lifecycle bindings admitted from device runtime facts, while daemon request/session policy and public response construction stay put. Based on main, which already carries the boot unit, the parametrized cutover gate and the apps unit. Readiness is package-owned there, so the Apple and Android bindings call ensureAppleReady/ensureAndroidReady rather than a root readiness bag; ensureAppleReady gained an onColdBootStart hook so open keeps warming the runner cache in parallel with a cold boot, and a narrow markBooted port publishes readiness' fresh observation so a flow still makes one simctl listing. Cutover rows take R24-R27, clear of the accepted catalog and the sibling install stack, and cutoverTableDefects rejects a duplicate rule id. Two defects this unit introduced are fixed here rather than shipped: `open <app> <url>` dropped the URL on a first open, and test-IME activation was first fatal on an unobtainable helper and then over-caught. Helper unavailability is a typed non-activation outcome now; fence, lock and post-record failures propagate. The duplication the unit had accumulated is gone: one runtime-admission module instead of five per-command copies, one direct-lifecycle binding factory instead of six hand-rolled packages, one transport-hint predicate, one session finalization path, and no identity-wrapper module. * fix: allocate lifecycle cutover rows after deployment * chore: preserve lifecycle union reconstruction * fix: reconcile lifecycle runtime stack * refactor: tighten lifecycle runtime topology * refactor: remove superseded runtime adapters * fix: preserve stacked runtime cutovers * test: preserve migrated runtime ownership * test: move Android deployment retry ownership * test: extract runtime hint fixtures * fix: preserve lifecycle stack invariants * fix: complete lifecycle runtime cutover * fix: remove lifecycle cutover residue |
||
|
Michał Pierzchała
|
66cca1a5b8 |
refactor: route install commands through platform runtime (#1758)
* refactor: route install commands through platform runtime * fix: preserve stacked runtime facts * fix: align deployment facts with shutdown runtime * refactor: simplify capability facts projection * style: format capability facts projection * fix: preserve migrated capability ownership * fix: propagate deployment artifact cancellation * refactor: move Harmony deployment mechanics into package * refactor: move Apple deployment tools into package * refactor: move Android deployment tools into package * refactor: inject deployment temporary storage * refactor: remove superseded deployment helpers * fix: preserve provider deployment transport |
||
|
Michał Pierzchała
|
5855dfc2e0 |
refactor: route shutdown through device runtime (#1757)
* refactor: route shutdown through device runtime * fix: cover shutdown cutover review gaps * fix: propagate shutdown cancellation * fix: move shutdown mechanics to platform owners * fix: pass device to shutdown fact fixture * test: cover shutdown facts in session state fixtures * test: simplify Android shutdown assertions * fix: preserve Apple shutdown cancellation |
||
|
Michał Pierzchała
|
39cd4d346a |
refactor: route appstate through platform runtime (#1755)
* refactor: route appstate through platform runtime * test: keep appstate capability fixture below complexity limit * test: cover appstate required readiness fact * fix: align appstate facts with boot readiness * fix: keep appstate use declaration minimal * fix: close appstate parity and ownership gaps * docs: record final appstate size accounting * fix: merge neutral runtime imports * docs: align final appstate size totals * fix: remove stale runtime dependency edges * docs: correct appstate size accounting * refactor: keep runtime-use factory internal * docs: itemize runtime-use relocation * fix: move appstate queries into runtime packages * refactor: retire root foreground query paths * refactor: share Android foreground parser ownership * fix: preserve Android appstate parser precedence * docs: keep appstate evidence in review artifacts * fix: keep appstate runtime loading lazy * fix: fail closed for stale limrun appstate * fix: preserve limrun recovery and abort appstate * fix: narrow limrun exact-owner recovery * fix: allocate appstate cutover rule * fix: reconcile appstate with merged main * style: format harmony runtime test * fix: allocate appstate rule id * fix: allocate appstate layering rule * fix: remove stale app command admissions * fix: close appstate layering regressions * fix: align Harmony capability parity with runtime facts * test: cover limrun recovery-only readiness * fix: keep Limrun recovery binding app-log only * fix: parse Android app state in linear time |
||
|
Michał Pierzchała
|
9c22467832 |
refactor(ci): make gate ownership structural (#1429) (#1753)
* test(ci): prove every registered gate is owned and reachable (#1429) A check that silently stops running looks exactly like a green build. Two suites had already stopped: `check:tmpdir-leaks` (with its model tests) and `test:fixture-cache` are real package scripts that no workflow ran, reachable only through the `check:unit` aggregate CI never invokes. `CHECK_CATALOG` becomes the registry of every check and `pnpm gate <id>` the only way CI runs one, so finding what a lane runs is a scan for `pnpm gate` rather than an attempt to interpret shell. `pnpm check:gate-manifest` then asserts against the real workflows that every registered check is run by some qualifying lane (per unit, not per script name), that every check the real selector activates for a path is run by a lane that path would start (#1420's class), and that every Vitest project and suite script belongs to a check. The wiring that keeps those honest is asserted too: a gate id must name a registered check, an `if:` must be ruled on in GATE_CONDITIONS so `if: false` unowns what it guards, an action declared to run a gate is proven to, and a job whose steps the loader cannot open fails closed. It deliberately does not try to prove CI runs project code only through `pnpm gate`. Whether a shell block executes project code is not decidable from its text, so shell this model does not recognise earns no ownership credit — the failure direction is a check reported unowned, never one waved through. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ * test(ci): update the two suites that assert on rewired workflow text `scripts/mutation/workflow.test.ts` and `test/ci/trusted-fixture-artifact.test.mjs` read the workflow and action files and assert on their command text, so routing those steps through `pnpm gate <id>` moved what they were matching. They are the two suites the manifest cannot help with: it proves a gate is still run, not that a test asserting on how CI spells a command was updated with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ * fix(ci): credit gates by execution shape, and keep every guard Three ways the manifest could report a gate as owned when it does not run. 1. Crediting was a substring scan over `run:`, which #1429 explicitly rules out — "do not infer reachability from a command name merely appearing in workflow text". `false && pnpm gate x`, a gate inside `if false; then … fi`, one named in a heredoc, and `echo pnpm gate x` all credited it. There is a live instance: conformance-regenerate.yml's "Fail if regeneration changed anything" step names `pnpm gate maestro-regenerate` inside an error message telling a human to run it, and that credited the gate. A gate now counts only as the first command segment of a line, and a body carrying shell structure earns nothing. Reachability inside a script is not decidable, so this does not try: unrecognised shape means no credit and the check reports unowned. `VAR=$(pnpm gate x …)` is read, since the assignment form is unambiguous and the gate runs. 2. Job-level `if:` was not modelled at all, though six live jobs carry one, so a job that cannot run still credited every gate inside it. Two conditions on the mutation lanes are now declared. 3. A caller's `if:` REPLACED the guard on a nested composite-action step (`guard[0] ?? step.condition`), so an outer `always()` erased an inner `if: false`. Steps carry every guard between the lane and the step. Also corrects two source comments that still claimed project code run outside the runner fails the manifest. It does not: such a step earns no credit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ * ci: add the run-gate action that names a gate structurally The seam the ownership proof will read instead of shell. A lane says which gate it runs in `with.gate`, a typed input the manifest reads straight out of the YAML and validates against CHECK_CATALOG. Nothing here is wired yet — the ~60 call sites and the model change follow. Added first so the target of that conversion is reviewable on its own. `args` cannot select which gate runs; it is appended after the id, so the worst a wrong value does is fail the gate it already named. There is no `|| true` and no output capture: the gate's exit code is the step's exit code, so a gate cannot run without being able to fail its lane. Part of #1429. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ * merge: main (#1770) and route its three new steps through the runner #1770 landed the orphan-check fix on main, wiring `check:tmpdir-leaks`, `check:tmpdir-leaks:test` and `test:fixture-cache` into Coverage, Layering Guard and Integration Tests. This branch had wired the same three through `pnpm gate`, so the merge produced two steps per check rather than a conflict — each check ran twice. Kept main's steps, with the placement and reasoning reviewed on #1770, and changed only their `run:` line to the canonical runner. Dropped this branch's duplicates. Net effect on CI is unchanged: the same three checks, in the same three lanes, once each. Gate manifest green after the merge: 47 checks wired across 33 lanes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ * fix(ci): address review — suite detection, freerange, glob, vacuous skip-list Six review findings plus the mutation blocker. [bug] `registered` was shape-only, so a `test:*` script running `node src/bin.ts test <dir>` resolved to a `script:` leaf and was invisible. Four `test:replay:*` scripts were owned only because someone hand-registered them; `test:replay:android` was neither registered nor reported while the nightly ran the same six .ad files by inlining them. A `test:*` script is now a suite by name. `replay-android` is registered, and the nightly runs the script instead of re-listing its files so the two cannot drift. The nightly invokes it inside `reactivecircus/android-emulator-runner`'s `script:` input — shell handed to a third-party action this loader does not read — so the suite executes but cannot be credited. Recorded in UNPROVABLE_OWNERS with that exact reason rather than assumed. The fixed detector also found a second orphan the review did not name: `test:integration:progress`. That one is a reporter whose `--check` sibling is the registered gate, so it is declared in REPORTING_SCRIPTS — a declaration that itself fails when inert. [bug] `freerange` defaulted to localRunnable, so fail-open ran `fr` (a Bun binary) on the pre-push path. Now false. [suggestion] The `--run` skip-list asserted `build:android-snapshot-helper`, a name `android-helpers` no longer uses, so it could not fail. Derived from the catalog instead. [suggestion] `matchesGlob` joined `**` splits with `.*`, making the adjacent slash mandatory — GitHub's `**` matches zero directories, so `src/**/*.test.ts` did not match `src/a.test.ts`. Pinned against `packages/*/src/**/*.test.ts`. [suggestion] Deleted the unwired `run-gate` action. It had no callers, was absent from GATE_ACTIONS, and its comment described a system that had not shipped. It returns with the rewiring, not before. [suggestion] Collapsed the module headers that narrated discarded designs. Mutation: `daemon entrypoint publishes HTTP metadata and cleans up on shutdown` is the only test here that spawns a real daemon process. It takes ~1.1s alone but exceeds Vitest's 5s default inside Stryker's dry run, which aborts the sweep before a single mutant runs. Given 30s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ * fix(mutation): order sandbox aliases longest-first so subpaths resolve Every shard of the mutation sweep aborted in Stryker's dry run with: Cannot find package '@agent-device/selectors/engine' imported from .tmp/stryker/sandbox-*/src/core/selector-pipeline.ts The alias was generated correctly; it just never won. Vite matches a STRING alias by prefix and takes the first hit, and `workspaceSpecifierTargets` emitted the bare `@agent-device/selectors` ahead of the subpath entries. The bare entry therefore captured `@agent-device/selectors/engine` and rewrote it to `…/src/index.ts/engine`, which does not exist; Node fell back to real package resolution, could not find the subpath inside the sandbox, and the dry run failed before a single mutant ran — so the shard uploaded an empty envelope instead of a report and the ratchet failed for want of one. Sorting longest specifier first makes the most specific alias win: @agent-device/selectors/engine -> packages/selectors/src/engine.ts @agent-device/selectors/ast -> packages/selectors/src/ast.ts @agent-device/selectors -> packages/selectors/src/index.ts `/ast` never tripped this because nothing in a related test set imported it; `selector-pipeline.ts` introduced the first subpath import that mattered (#1744), so the mutation lane has been unable to run since that landed. Any PR touching `scripts/mutation/**` — which fails open into the full sweep — would have hit it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ * refactor: derive gate ownership from workflow structure * fix: run gates without optional arguments * fix: resolve mutation workspace subpaths exactly --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
c7565cb1f8 |
refactor(snapshot): clean snapshot ownership (#1754)
* refactor(snapshot): clean snapshot ownership * fix(snapshot): address ownership review feedback |
||
|
Michał Pierzchała
|
eabc936a0f |
refactor: route apps through request runtime (#1756)
* refactor: route apps through request runtime * test: remove stale apps adapter mock * fix: clean apps runtime replay artifacts * fix: remove stale runtime test exports * refactor: simplify apps runtime admission * test: exercise runtime use through facade * fix: close apps runtime admission gaps * fix: disambiguate doctor app inventory callback * fix: keep capability fixture below complexity limit * fix: keep HarmonyOS app inventory fail-closed * test: type HarmonyOS app admission fixture * fix: restore HarmonyOS app inventory parity * test: align HarmonyOS readiness fixture * fix: preserve HarmonyOS doctor app parity * test: type HarmonyOS doctor fixture * fix: isolate HarmonyOS doctor policy * fix: align apps cutover with shared rule catalog |
||
|
Michał Pierzchała
|
b13c06338e |
refactor: route boot through readiness runtime (#1747)
* refactor: route boot through readiness runtime * fix: separate boot admission from readiness * fix: register boot cutover policy * refactor(runtime): move readiness into platform owners * fix(test): tolerate provider temp cleanup races |
||
|
Michał Pierzchała
|
ba48146520 |
refactor(layering): one parametrized runtime-command-cutover gate (ADR 0019 §8) (#1745)
* refactor(layering): fold the four cutover policies into one parametrized gate ADR 0019 §8: the per-command cutover gates consolidate into one parametrized runtime-command-cutover gate driven by a table of migrated commands. Adding a migrated command adds a row; the mechanism carries one planted-red proof instead of one per command. Part of #1739 (wave 0) * fix(layering): scope cutover calls to lexical owners * chore: format cutover ownership model |
||
|
Michał Pierzchała
|
8f98d23f14 |
refactor(layering): give each colliding rule id its own number (#1750)
* refactor(layering): give each colliding rule id its own number
R11 and R13 each named two unrelated rules. report() groups violations by the
rule string and titles every annotation `Layering drift (${rule})`, so a shared
number made the guard's output ambiguous about which rule fired.
Reference counts decided which rule keeps its number. R11 package-boundaries is
named in ~30 places (CONTEXT.md, ADR 0019, testing.md, the mutation and
affected-check configs, four package source comments, its own tests) against one
for the contracts rule; R13 platform-package-substrate is the RULE in three
policy files plus CONTEXT.md, ADR 0019 and model.ts against two for the devices
cutover. Both keepers stay put and the two newest rules move up:
R11 contracts-implementation-authority -> R18
R13 device-inventory-cutover -> R17
R17/R18 follow the namespace's order-of-addition convention (R14 #1701 < R15
#1702 < R16 #1724): device-inventory-cutover landed in #1699 and
contracts-implementation-authority in #1701. #1656 took R19 for
selector-pipeline-ownership on the same reading.
The rule-map header in check.ts is renumbered and reordered back into numeric
order, and gains the R18 entry the contracts rule never had -- without it a
reader looking up an R18 violation finds nothing where they used to find the
wrong rule. deviceInventoryCutoverSummary() was also the only OK-line summary
not leading with its rule number, which is what made the number unreadable from
the success line in the first place.
Also corrects a normative ADR reference. ADR 0019's platform-package import
rules -- contracts-to-platform, sibling-platform, root/daemon, raw-process --
are R13's, as CONTEXT.md:420 already says. The R11 attribution predates
platform-package-policy (#1697, a day before #1699), when R11 was the only
package rule.
* chore(layering): retire the expired R11/R13 collision allowances
KNOWN_RULE_ID_COLLISIONS was opened for exactly the two collisions the previous
commit renames apart, and ruleIdCollisionFailures expires an allowance on
contact: once the collision is gone the entry fails as stale, because a list
still naming it would wave it back through if anyone reintroduced it.
Both entries are therefore deleted in the change that removes the collisions,
leaving the empty list that admits nothing. The namespace is now one-to-one
across R2-R19.
|
||
|
Michał Pierzchała
|
7f5dbd2e50 |
chore: drive unused production exports to zero (#1743)
* chore: drive unused production exports to zero `pnpm check:production-exports` has been failing on main with 21 findings. Each was investigated rather than blanket-suppressed; they split three ways. Genuinely dead, deleted: - `androidDeviceForSerial` (android/adb.ts) had zero references anywhere, tests included. - `streamAndroidLogcatWithAdb` (android/logcat.ts) had no production consumer and only a guard-clause test; its `captureAndroidLogcatWithAdb` sibling is the published SDK surface. Removed with its options type and test. Test-only aliases over live siblings, collapsed: - app-log-resource-store re-exposed four bound store methods; production used only `resolvePath`, tests used the other three. The sibling screen-recording-resource-store exports just the store, so this now matches: one export, all consumers call `appLogResourceStore.x`. - device-claims re-exported `canonicalLocalDeviceKey` for a single test, while production imports it from device-claim-paths directly. Dropped the re-export and pointed the test at the canonical module. Real consumers the analysis cannot see, exempted with the reason: - The nine remaining `src/cli/commands/*Command` handlers are reached only through `dedicatedCliCommandHandlerLoaders`, the dynamic import() table in router.ts. `deviceCommand` already carried an inline suppression for exactly this; replaced it with one config entry naming the table that enumerates all ten, matching the existing daemon route-handler entry. - `resolveVitestMaxWorkers` (vitest.config.ts), `DEVICE_CLAIM_IN_USE_SAMPLE` (bench sample producers) and the capture-kit `createAppLogLiveHandle` facade export joined the existing entries that already record their exact shape. - `**/*.fixtures.ts` is now an ignorePattern: all 15 build doubles for co-located tests, several import `vi`, and none is imported by production source. Pattern-matching them as test infrastructure also keeps this class of finding from recurring. Gate now reports zero. Unit suite, layering, fallow audit, MCP metadata, build, bundle-owner and package checks all pass. * chore: scope the fixture exemption to unused exports Review feedback on #1743: `ignorePatterns` removes a file from every Fallow mode and rule, but the false positive here is only production-unused-exports. Moved *.fixtures.ts to an ignoreExports entry so fixtures stay inside health, dead-code and cycle analysis. Kept `exports: ["*"]` rather than today's three symbols because the property is per-file — no fixture module has a production consumer — so a new fixture symbol should not reopen the finding. check:production-exports still reports zero, and a full `fallow --summary` returns identical totals (2 dead-code / 4 dupes / 123 health) with and without the change, so nothing was newly surfaced or newly hidden. * test(fallow): prove fixture policy scope |
||
|
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
|
057ab1c82d |
fix(layering): stop double-reporting contracts-authority violations (#1746)
* fix(layering): stop double-reporting contracts-authority violations main()'s violation list spread checkContractsImplementationAuthority(sources) twice, so every R11 contracts-implementation-authority finding was printed and ::error-annotated twice on a red run — inflating the headline violation count and producing duplicate GitHub annotations on the same file:line. Verified by planting a `setTimeout` call in a contracts production source: the rule reported 2 identical violations before and 1 after, with the extra annotation gone. `pnpm check:layering` stays green (136/136 policy tests). Nothing in the suite covers main()'s assembly of the violation list — the policy tests all call their rule functions directly — so neither a duplicated nor a dropped entry there is currently detectable. * test(layering): hold main() to wiring every rule exactly once The duplicate this branch removed survived because nothing enumerates the guard's rules: main()'s violation list is hand-written, and the per-policy tests call their rule functions directly, never seeing the wiring. A lost spread is the dangerous version of the same gap — the rule stops being enforced and the run still prints OK. Make the file's own bindings the oracle: every in-scope `check*` value, local or imported, must be spread into main()'s violation list exactly once. That covers both directions plus a third case — a policy written and never wired in. Fails closed if main() or the array is renamed, so the instrument cannot pass by finding nothing. Test-only rather than an R17 inside the guard: a self-referential rule is defeated by dropping its own spread, which is exactly the failure it exists to catch. Verified by mutating the real check.ts in both directions (re-planting the duplicate, then dropping checkZeroDepJobs) — each turns the run red, and the restored file is green at 143/143. * test(layering): discover layering suites by glob instead of by hand check:layering named its 14 test files one by one, so adding a policy test meant remembering to register it — and twice nobody did. Both halves of the R16 record cutover shipped with tests that have never run: scripts/layering/record-runtime-mechanics-policy.test.ts (2 tests) scripts/layering/record-runtime-registry-policy.test.ts (1 test) Their policies are live in the guard; only the tests were dormant. All three pass, so nothing had rotted — the coverage was simply never being collected. Glob the directory the way mutation:test already globs its own, which makes the filesystem the enumeration and retires the registration step. 143 -> 146 tests, still green. This is the same defect as the duplicate spread this branch opened with, one level up: a hand-maintained list with nothing checking it against reality. * refactor(layering): register guard rules in a keyed table Replaces the AST wiring guard with a construction that cannot express the defect, per review on #1746. The parser was the wrong instrument: it reconstructed one array's shape from TypeScript syntax, so it only recognised top-level function declarations and imports whose local name matched /^check[A-Z]/. A const-defined or aliased rule was invisible to it, a helper named checkX was a false positive, and naming and syntax became part of the interface — all to detect a mistake rather than prevent it. Rules now live in a keyed table over a shared context, executed once via Object.values. An object cannot hold a key twice, so double registration is unrepresentable rather than merely detected, and oxlint's no-dupe-keys rejects the attempt at the source. LayeringRuleId makes a missing key a type error, and LAYERING_RULE_IDS gives the catalog to check exhaustiveness against. Call sites and order are unchanged, so grouped output and the success line are byte-identical. One regression test remains, through the production interface: scripts/ is outside tsconfig.json's `include`, so the Record's exhaustiveness is an editor signal rather than a CI gate, and the catalog assertion is what fails the build when wiring goes missing. Verified by mutation: dropping an entry and registering an uncatalogued one both fail the test, a duplicated key fails oxlint, and re-planting the original contracts violation reports it exactly once. Net -133 LOC. |
||
|
Michał Pierzchała
|
f5d9789764 |
feat: enforce local device claims and reconcile stale owners (#1735)
* feat: enforce local device claims * fix: address device claim review feedback * fix: persist canonical daemon claim state directory |
||
|
Michał Pierzchała
|
b8dd6a5854 | refactor: tighten capture ownership boundaries (#1736) |