mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
docs/improvement-audit-tracker
90 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michał Pierzchała
|
c77bc40d48 |
refactor(daemon): Wave 6 — migrate clipboard, app-switcher, trigger-app-event, settings, alert, react-native and capabilities onto request-bound runtimes (R55–R63) (#2021)
* refactor(daemon): migrate clipboard onto request-bound runtimes (R55) Wave 6 unit 1 of the ADR 0019 platform-free daemon migration (#1739). `clipboard` leaves the legacy dispatch projection: admission is now the action-selected `readClipboard`/`writeClipboard` fact the parsed subcommand names, and the only execution is that one bound operation. - new `@agent-device/contracts/clipboard-runtime` facet, riding the existing `Interactor` seam through `interactor-operation-binding.ts`; read and write are separate cells because a provider can genuinely expose one half only. - every owner states its own cells: Apple gains a `system/` facts module (simulator or the macOS host, matching the retired `supportsHostOrSimulatorSurface` closure), Android admits every real kind, Linux the desktop device, and HarmonyOS/Vega/web refuse -- none ever carried a bucket. Limrun reuses the local Android interactor and refuses on iOS; WebDriver rides interactor reachability like `back`/`home`. - retires the `core/dispatch.ts` clipboard arm and handler, the descriptor's capability bucket and `dispatch` leaf, and the Apple plugin's clipboard admission closure. `handlers/session.ts` loses its inline handler (and its last `dispatchCommand`/`requireCommandSupported` imports) to the new `handlers/session-clipboard.ts`. - `bindLocalInteractorOperationSet` collapses the byte-identical local interaction bind list Android and Linux each held a copy of. Cutover row R55 with its retirement, admission-member and single-bind claims. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * refactor(daemon): migrate app-switcher onto request-bound runtimes (R56) Wave 6 unit 2 of the ADR 0019 platform-free daemon migration (#1739). `app-switcher` leaves the legacy dispatch projection: admission is the owner's `appSwitcher` fact and the only execution is that one bound operation, resolved by the generic route alongside back/home/orientation/tv-remote. - new `@agent-device/contracts/app-switcher-runtime` facet on the shared `Interactor` seam, bound through the interactor catalog. - Apple states one springboard reading for `home` and `app-switcher` (parity: the retired `supportsAppAndDeviceLifecycle` closure gated both off the same per-AppleOS row, so macOS and watchOS refuse); Android admits every real kind; HarmonyOS admits both kinds, restating the retired overlay membership; Linux/Vega/web refuse. Limrun reuses the local Android interactor and refuses on iOS; WebDriver rides interactor reachability. - retires the `core/dispatch.ts` arm, the capability bucket, the `dispatch` leaf, `HARMONYOS_SUPPORTED_COMMANDS` membership, the Apple plugin closure, and the now-readerless `appAndDeviceLifecycle` row in the per-AppleOS table. - router tests that used `app-switcher` as their legacy-dispatch stand-in move onto bound operations; the typed-error `supportedOn` test moves to `perf`, the one command that keeps a capability-matrix row after this wave. Cutover row R56 with its retirement, admission-member and single-bind claims. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * refactor(daemon): migrate trigger-app-event onto request-bound runtimes (R57) Wave 6 unit 3 of the ADR 0019 platform-free daemon migration (#1739). `trigger-app-event` leaves the legacy dispatch projection: admission is the owner's `triggerAppEvent` fact and the only execution is that one bound operation. The split follows ADR 0019 §2 — a facet input names no command, request, or CLI flag. The event name pattern, the payload size limit, and the per-platform `AGENT_DEVICE_*_APP_EVENT_URL_TEMPLATE` are daemon policy and stay in `core/app-events.ts`; what reaches the owner is a resolved URL to open. They also stay downstream of admission, where the retired `dispatchCommand` ran them, so an unsupported device still reports its unsupported cell rather than an argument error. - new `@agent-device/contracts/app-event-runtime` facet on the shared `Interactor` seam, bound through the interactor catalog. - Apple admits every leaf with a constructible interactor (no closure ever gated this command beyond its bucket), Android every real kind, and Linux/HarmonyOS/Vega/web refuse. It is the one system leaf both Limrun legs serve, since each implements `open`; WebDriver rides interactor reachability. - retires the `core/dispatch.ts` arm and handler, the capability bucket, the `dispatch` leaf, and the session route's last capability-gate-then-`dispatchCommand` thunk: every leaf on that route now supplies a bind-and-execute thunk. - the end-to-end delivery tests keep their shell-level assertions and move onto the migrated composition. Cutover row R57 with its retirement and single-bind claims. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * refactor(daemon): migrate settings onto request-bound runtimes and retire the legacy dispatcher (R58) Wave 6 unit 4 of the ADR 0019 platform-free daemon migration (#1739). `settings` was the last `DISPATCH_HANDLERS` arm, so this change closes the command and retires the legacy command dispatcher whole. - new `@agent-device/contracts/settings-runtime` facet on the shared `Interactor` seam, bound through the interactor catalog. What reaches the owner is its own settings vocabulary (setting, state, resolved app id, typed coordinates); the CLI parse, the macOS setting-name gate, the clear-app-state app-id check and the coordinate typing are daemon policy and stay daemon-side, downstream of admission where the retired leaf ran them. - Apple shares clipboard's exact host-or-simulator reading (the retired admission intersected the `settings` bucket with the same `supportsHostOrSimulatorSurface` closure); Android admits every real kind; HarmonyOS matches its retired overlay membership; Linux/Vega/web refuse. Limrun splits Android-reuse / iOS-refusal like `app-switcher`; WebDriver refuses unconditionally, since its interactor declares settings unsupported. - retires `dispatchCommand`, `dispatchWithInteractor`, `dispatchKnownCommand`, `DISPATCH_HANDLERS`, `listRegisteredDispatchCommandNames`, and the request router's `executeGenericPlatformCommand` fallback. `core/dispatch.ts` keeps only `dispatchGestureViewport`, whose last consumers are replay/test. Retiring the dispatcher surfaced two callers broken since Wave 5 moved `press` onto a bound runtime: react-native overlay dismissal and the opt-in interaction no-change retry both called `dispatchCommand(device, 'press', …)`, which has thrown `INVALID_ARGS: Unknown command: press` on main since R48. Both now run the same bound `tapPoint` every other touch leaf uses. The retry declares its own callback seam rather than importing runtime admission, so the policy stays readable without the binding stack — and that inversion, plus the dispatcher's retirement, drops the largest type-level import cycle from 25 files to 21. Cutover row R58 with its retirement and single-bind claims. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * refactor(daemon): migrate alert, react-native and capabilities onto facts (R59/R61/R63) Wave 6 units 5, 7 and 9 of the ADR 0019 platform-free daemon migration (#1739), plus the residue reclassification the tracker asks for as an analysis task. R59 `alert` — new `@agent-device/contracts/alert-runtime` facet with four action-selected legs (`readAlert`, `awaitAlert`, `acceptAlert`, `dismissAlert`) on the shared `Interactor` seam. The daemon route admits and binds exactly the leg the parsed subcommand names, and the poll and retry windows move to the owners with it: how long a transient sheet takes to appear, and how many times to re-ask a runner that says it is not there yet, are family mechanics, not request policy. `src/platforms/apple/alert.ts` now holds the Apple windows verbatim (with the macOS-helper / XCTest-runner split), and Android's legs read the same presented tree `snapshot` publishes, which is why their occlusion reading still holds. Apple's cell is the retired `supportsAlertSurface` closure restated as facts — the host-or-simulator reading widened by physical iOS — and that closure was the per-AppleOS capability table's last reader, so `src/platforms/apple/capabilities.ts` goes with it. R61 `react-native` — the command's device work moved onto a bound `tapPoint` with R48; this retires the capability gate that still stood in front of it and moves admission ahead of the observing capture, so an owner that cannot dismiss an overlay refuses without first spending a snapshot on it. That exposed a real defect: the request handler chain never forwarded the request's runtime bindings to this route, so the dismissal leg had been reaching a missing gateway ever since R48 — only the no-overlay-detected path returned early enough to hide it. Fixed, with a chain-level regression test. R63 `capabilities` — the projection now reads each command's own declared `platformExecution` uses instead of a hand-written map plus a "no capability bucket means supported everywhere" fallback. That fallback is what let a stopped Android AVD advertise `snapshot press fill` it cannot run, and a Vega VVD advertise every migrated command; both collapse to the fact-derived set here. The command itself executes nothing on a device, so it declares `none`. Residue: `batch`, `debug` and `events` reclassify to `none` — each reaches no device and delegates nothing that does. `replay`/`test` keep their gesture viewport and boot-diagnostics edges, `daemon`/`web` hold platform imports in their own CLI modules, and `react-devtools` still injects device-runtime `runtime`, so all five stay `legacy`. Cutover rows R59 and R61 with their retirement and single-bind claims. Descriptors: 32 legacy at the wave checkpoint, 9 now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * fix(daemon): restore two settings/alert sequences the migration had shifted Self-review of the Wave 6 diff against `origin/main` found two places where the migrated routes were faithful in what they did but not in when: - `settings` typed its location coordinates before expiring the ref frame, so a request that failed on a bad coordinate no longer expired it. The retired route expired the frame first, then emitted its diagnostic, then typed the coordinates inside the leaf. Same order again. - `alert` narrowed a frontmost-app session to "no bundle" in the daemon, which also stripped the bundle from the XCTest runner leg. That narrowing was only ever the macOS helper's, and it already lives in `platforms/apple/alert.ts`; the runner leg gets `session.appBundleId` unconditionally again, pinned by a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * fix(daemon): address adversarial review of the Wave 6 cutovers Three independent reviews (behavior parity, correctness, ADR 0019 conformance) ran against the branch. What they found, and what changed: Correctness - The R48 retry seam was unreachable. `captureSnapshot` builds it from the request's runtime bindings, but no caller forwarded them, so every retry resolved to a skip. The `snapshot` route now threads `inspectFacts`/ `bindDevice` through `createSnapshotRuntime` and the daemon snapshot backend down to the capture. - A retry tap that rejected escaped the capture it was decorating and turned a plain `snapshot` into an error. It is caught and reported as a skip, matching what the seam's own contract already claimed. - The attempt is spent before the device work again, as the retired route did, so an owner that fails mid-flight cannot be re-attempted from a full budget. - `react-native dismiss-overlay` reached its required `tapPoint` through `?.` and answered `dismissed: true` when the operation was absent. It refuses. - `factOwnedCapabilityAvailable` indexed the facts map unguarded, and treated an empty `required` as proof (`[].every` is vacuously true). Both fail closed. ADR 0019 conformance - §6 forbids a `none` descriptor from binding a device, and `capabilities` bound three times to answer `logs`/`network`/`record`. Every owner composes a binding's facts with the same function `inspectFacts` calls, so those probes read back values the single inspection already carries — at the cost of a device claim on a read-only query. They are gone, and with them the last three empty-`required` admission uses. - §9 is one admission per handler; the retry tap re-admitted on every retry round. It memoizes per device. - `installFamilyCapabilityAvailable` was scaffolding this wave was scheduled to retire: the general projection returns the same verdict for all four install-family commands. Deleted. Leftovers the cutovers created - `requireCommandSupported` lost its last production caller when R56 migrated `app-switcher`: every generic-route command is admitted from owner facts before the dispatcher runs. The dead arm, the function, and `commandUsesDeviceRuntimeExecution` are removed. - `CommandDispatchFacet`, `descriptor.dispatch`, and `explain`'s `dispatch=` field described a dispatcher R58 deleted. - `request-router-android-modal.test.ts` asserted on a `dispatchCommand` mock whose module export no longer exists, so three assertions were vacuous. - `generic-route-runtime-completeness.test.ts` now exists — a comment claimed it did. It pins the routing table as total over the generic route. - Comments and test names describing the retired dispatcher, the deleted AppleOS capability table, and a react-native regression that never shipped. Also records two deliberate provider cell changes the migration made (physical Apple `clipboard` admitted, provider `alert` refused) and the react-native widening to Linux, web and HarmonyOS, and drops a scratch probe file that was committed by accident. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * fix(apple): type the alert-absence retry instead of matching error prose Review blocker 1 on #2021. The Apple alert legs decided retry and hint eligibility by substring-matching error messages for "alert not found" / "no alert", and `alert wait` swallowed *every* read failure. A dead runner, an unreachable macOS helper or a canceled request was therefore spent as poll budget and finally reported as `alert wait timed out`, hiding the real cause. Both backends now state absence as typed evidence: - The XCTest runner answers `ErrorPayload(code: "ALERT_NOT_FOUND", ...)`. It is diagnostic-only, so it stays `COMMAND_FAILED` on the wire and surfaces as `details.runnerErrorCode` — the same shape `RUNNER_BUSY` already used. - The macOS helper adds `reason: "alert-not-found"` to its JSON error details, which the helper client already forwards verbatim. `isAlertNotFoundError` reads only those two fields. `awaitAppleAlert` re-throws anything that is not a typed absence instead of polling through it, and the scoped-snapshot fallback hint attaches to typed absence alone. The three tests the review asked for, plus coverage the daemon-altitude copies could not express: a non-absence failure propagates immediately from `wait`; an action does not retry a failure whose message merely reads like an absence; the macOS helper's typed reason is retried like the runner's. The daemon-level non-absence test moved to the family suite that owns this policy since R59, lowering that file's size pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * fix(runtime): admit clipboard and provider operations from what execution checks Review blocker on #2021, reproduced on a Pixel 9 Pro XL / Android 36 emulator: `capabilities` advertised `clipboard`, then `clipboard read` failed with `UNSUPPORTED_OPERATION: Android shell clipboard read is not supported on this device.` Admission and execution were consulting different authorities, which ADR 0019 §2 forbids — a bound operation must already be admitted. Android. `cmd clipboard` has no shell implementation on every build, and the retired bucket admitted both halves on every real Android kind, leaving the leaf to discover the refusal after the fact. Support is now a fact: the owner probes once per device (cached for its lifetime — a build's shell command set cannot change while the device is up) and states `owner-capability-missing` when adb names the condition. The probe is definitive in one direction only: adb saying so means unsupported, a probe that cannot run means unknown, and reporting unknown as unsupported would hide a working clipboard behind a transport hiccup. The predicate moves to `@agent-device/contracts/android-clipboard-support` so admission and the leaf's own defense-in-depth check cannot drift apart. Cost, stated plainly: the first facts inspection per device now spends one adb round trip, including for requests that never touch the clipboard. WebDriver. `webdriver-interactor.ts` refuses through `capabilitySupported`, while fact generation admitted from interactor reachability alone — so a provider configured with `capabilityOverrides: { 'clipboard.read': 'unsupported' }` was admitted and then thrown out of. The declared capability map is now an input to fact generation, and the refusal carries the map author's own note. Applied to every operation with an unambiguous capability key, not just the two named in review: the mechanism is identical and a half-applied fix would leave the same defect for `back`/`home`/`orientation`/`tap`/`fill`/`type`/`scroll`. Behavior is unchanged by default — every one of those is `supported` or `partial` in the base map — so only an explicit override bites. `focus`, the gesture tiers and `trigger-app-event` keep reachability: no capability key maps to them 1:1. Also collapses the eight identical `*RetiredDispatchProjectionProof` wrappers in the cutover table into one parameterized factory (second review point). Parity tests: an Android build reporting either unsupported-shell phrasing, the probe cache, an adb failure staying admitted, and a WebDriver override refused at admission for each keyed operation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * refactor(layering): split the Wave 6 cutover rows into a sibling module Second review P2 on #2021. `runtime-command-cutover-table.ts` had reached 1,325 lines, past the point where one read covers it. Wave 6's eight rows move to `runtime-command-cutover-table-wave6.ts` and are spread back in, leaving the table at 1,095 lines. The split is by wave because that is how these rows are retired: a wave's rows are deleted together once the ADR declares its commands' migrations closed, and deleting a whole file is a cleaner end than excising a run of literals from the middle of a larger one. `retiredDispatchProjectionProof` moves to the shared extensions module, since both tables now use it — the main table for `snapshot`/`diff`, the sibling for its own eight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * fix(android): never fabricate clipboard availability from a failed probe Review blocker on #2021. The probe I added had a `catch { return true }`, then cached that result by device id for the runtime owner's lifetime. A transient adb offline or timeout therefore made `capabilities` advertise the clipboard on a build with no clipboard shell — recreating the exact lie the fix was for, and pinning it for the rest of the session. A test locked the behavior in. Support is now a typed verdict with three states, because "we could not ask" is not "it works": `supported | unsupported | probe-failed`. Only a definitive answer is cached; `probe-failed` refuses conservatively with a hint saying support could not be determined, and is deliberately not remembered, so the next inspection asks again. The same change repairs the ownership boundary. Turning raw adb stdout/stderr into a verdict is Android tool knowledge, so it belongs to the Android owner, not to shared vocabulary — `@agent-device/contracts/android-clipboard-support` now carries the typed union alone. The parser returns to `src/platforms/android/adb.ts` and runs in exactly one place, behind a new `AndroidToolHost.probeClipboardShellSupport` that hands owners the verdict. That also settles which Android home owns it: R13 lets only `src/platform-runtime.ts` import `@agent-device/platform-android`, so a parser shared between the package and the root leaf cannot live in the package either. Tests now cover the failure path the previous ones locked the wrong way: a failed probe refuses instead of admitting, its refusal says it could not determine support rather than claiming the build lacks it, and it is not cached — a second inspection re-probes and admits once the device answers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * refactor(contracts): declare each interactor operation once Second review P1 on #2021. `interactor-operation-catalog.ts` declared the same operation set three times — a name tuple, a complete local binder map, and a complete provider binder map — and each facet carried a mirrored `bindLocal…Interactor`/`bindProvider…Interactor` pair whose only difference was which interactor source to use and which label a refusal names. There is now one row per operation, carrying its facts key, its provider refusal label, and the facet's own executor. The local/provider split lives in the two adapters, which differ by exactly the thing that differs: the interactor source. Adding an operation is adding one row. Deleted: the parallel tuple, both binder maps, 32 mirrored wrappers across ten facet modules, and the per-facet `Local…`/`Provider…InteractorResolver` aliases that existed only to be re-exported. Kept: every facet's typed executor, now exported as its binding surface. Net −563 production lines in `packages/contracts`. Two consumers moved onto the catalog's public entry point rather than keeping a private path to a single operation: the app-event delivery test and the provider scenario fixture, whose two hand-bound keyboard legs are now whichever legs its facts admit. Each facet's tests spell out the composition the retired wrappers performed, so every assertion still exercises one executor reached through one source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * fix(android): let only a clean adb exit prove clipboard support Third review P1 on #2021. The typed verdict landed one layer too high. The adapter probe runs `adb shell cmd clipboard get text` with `allowFailure`, so a non-zero exit comes back as an ordinary result rather than a throw — and the only thing standing between that result and `supported` was the missing-shell prose check. A device that had gone offline, was unauthorized, timed out, or failed for any other reason produced none of that prose, so it fell through to `supported` and was then cached by device id for the runtime owner's lifetime. The `catch` I added guarded the one path adb almost never takes. Each adb outcome now proves only what it can: - `exitCode === 0` is the sole evidence of support, because it is the only result that shows the command ran. - The recognized missing-shell prose is the sole evidence of absence, and is read before the exit code — adb reports that condition non-zero, so checking the code first would turn every honest `unsupported` into a refusal. - Everything else — non-zero without that prose, and the transport throw — is `probe-failed`, which admission refuses and the cache does not remember. The package tests mocked the typed verdict, so they sat downstream of the bug and could not see it. The regression is therefore at the adapter, over the raw adb result: four planted reds (offline, unauthorized, device-not-found, generic failure) that all returned `supported` before this change, plus the two definitive verdicts and the ordering case that keeps `unsupported` reachable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * fix(android): never read adb's refusal prose out of the clipboard's contents Fourth review P1 on #2021, and a second instance of the same bug it names. The previous fix read `isClipboardShellUnsupported(stdout, stderr)` before the exit code. On a *successful* `cmd clipboard get text`, stdout is the clipboard's contents — arbitrary user text. Anyone who had copied "unknown command" or "no shell command implementation" (from a terminal, a bug report, this repo) had their own working clipboard classified `unsupported`, and the runtime owner cached that for its lifetime. Ordering prose ahead of the exit code to keep `unsupported` reachable traded one wrong admission for another. The exit code is decisive on its own when it is zero, so it goes first. Only a call that failed can carry prose about the call itself, which makes the missing- shell phrases meaningful on non-zero exits alone: if (result.exitCode === 0) return 'supported'; return isClipboardShellUnsupported(...) ? 'unsupported' : 'probe-failed'; `isClipboardShellUnsupported` now states that precondition, because reading it on a successful call is exactly the mistake to prevent. The same defect was already shipped in the helper's other caller. `runAndroidClipboardShellCommand` in `src/platforms/android/device-input-state.ts` has checked the prose before the exit code since #1950, so `clipboard read` on a clipboard holding either phrase threw `UNSUPPORTED_OPERATION` — telling the user their device does not support a clipboard it had just read correctly. It is not this wave's code and not reachable from the migration, but it is the same helper misused the same way, and documenting a precondition while leaving a caller that violates it invites the next regression. Repaired here, with the failure ordering otherwise unchanged: a non-zero exit still reports missing-shell as `UNSUPPORTED_OPERATION` and anything else as the adb result error. Both repairs are pinned by regressions that fail against the code they replace: four exit-0 cases at the adapter (verified red against the ordering this commit removes), and three at `readAndroidClipboardWithAdb` (verified red against `origin/main`) covering contents that look like a refusal, a genuine missing command, and an unrelated non-zero failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * fix(cli): bring the workflow help card back under its size budget `Coverage (2)` has been red on `main` and on every PR branched from it since #2020, which replaced three short Bootstrap lines with one longer line carrying the new selection semantics. It updated the content matcher for that line but not the size assertion beside it, so the card went to 9003 bytes against the `< 9000` both `cli-help.test.ts` and `cli-help-topics.test.ts` enforce. Nothing #2020 added is removed here — all of it is pinned by the matcher it shipped, and it is the sentence agents most need. The bytes come back from a clumsy repetition elsewhere in the card, where "settle" named itself twice in one clause: ... only when you did not settle, settle reported not settled, or ... ... only when you did not settle, it reported not settled, or ... which reads better short and puts the card at 8999. That is one byte inside the budget, which is the real finding: the card has no slack left, and the next sentence anyone adds re-opens this. The durable fix is a base-owner call between raising the budget and moving a block down into its sub-topic — the mechanism the card already uses, and which its own test documents. Flagged on #2021 rather than decided here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX * test(cli): raise the workflow help-card budget to 9100 The card is a curated agent-facing reference, and #2020 grew it for a good reason: the selection semantics it added are what an agent needs to predict which device a bare `open` picks. Holding that content to a limit set before it existed just moves the cost onto whoever writes the next sentence. 9100 is headroom, not a target. The previous commit left the card at 8999 of 9000 -- one byte -- which is not a state anyone should have to work in, and I had already established there is no slack left to reclaim: no trailing whitespace, and the only repeated runs are the deliberate column alignment in the Escalate footer. Trimming further would have meant deleting content the tests pin as load-bearing. This is explicitly interim. The card is ~9KB of dense prose in one string, and the real answer is to move a block down into its owning sub-topic -- the mechanism the card already uses and its own test documents ("Deep content moved out of the compact card, not deleted"). Raising the ceiling buys room to do that deliberately instead of under a red CI. Both enforcement sites move together, since they measure the same card through different surfaces: `cli-help.test.ts` reads it through the CLI, and `cli-help-topics.test.ts` through `usageForCommand`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RpfS12XApXqasuAWZJaEX --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
5b6feafe92 |
Extract snapshot policy from daemon to host-side facet (#1983) (#2014)
* refactor(snapshot): give the Wave 4 policies neutral host seams (#1983)
#2005 established the presentation ownership boundary and moved the iOS
presentation policies out of `src/daemon/`. It left the three remaining Wave 4
policies behind their existing daemon adapters. This closes that gap, so
`src/snapshot/` owns host-side snapshot policy generally rather than
presentation alone.
Freshness recovery: the window vocabulary, the Android staleness classification
and its thresholds, and the retry loop move to `src/snapshot/snapshot-freshness/`.
The loop is parameterized by a classifier and a retry schedule, so how long a
backend may lag behind a real transition is a policy input rather than a
constant the loop owns. `src/daemon/session-snapshot-freshness.ts` keeps only
what needs a session — reading and retiring the window on store-owned
`SessionState`, and choosing the comparison baseline from snapshot lineage — and
remains the declared R7 owner of `androidSnapshotFreshness`. The two call sites
#1739 named as the Wave 5 blockers, `selector-capture-runtime.ts` and
`deferred-interaction-outcome.ts`, now reach freshness through the seam.
Timeout evidence: whether a failure is the accessibility-timeout shape becomes a
policy in `src/snapshot/snapshot-timeout-policy.ts`. The published
`details.androidSnapshotTimeoutScreenshot` payload becomes vocabulary in
`@agent-device/contracts/snapshot-timeout-evidence`, built through constructors
so an assembly site cannot publish a fifth, undeclared arm. It gets its own
subpath rather than riding the shared capture facade, which keeps it out of the
CLI cold-start closure. Typed details, diagnostics and screenshot evidence are
unchanged.
Screenshot-overlay policy: which Android nodes earn an overlay ref, and what
rectangle an overlay covers, move to `src/snapshot/screenshot-overlay/`. The
daemon keeps approved artifact and ref assembly only — ranking, projection to
screenshot pixels, drawing and PNG IO.
The boundary test generalizes from the presentation subtree to the whole facet:
nothing under `src/snapshot/` may import `src/daemon/`. It gains a positive
control, because a filter that stopped matching would look identical to a
boundary being obeyed.
The residual call sites #1983 also named are audited and deliberately left in
place. `direct-ios-selector.ts` carries no presentation policy; its two pure
exports are selector derivation and ADR 0011 delegation-on-error, whose owner
would be the selector pipeline governed by R19, not this facet. ADR 0004 records
the finding so it does not have to be re-derived.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GLYhmt5ZNHQATG8T8ZFo7R
* refactor(snapshot): address adversarial review of the Wave 4 seams
Three findings from an adversarial pass over
|
||
|
Michał Pierzchała
|
775eddd749 |
feat: session-scoped echo protection for parameterized recorded inputs (#2013)
* feat: session-scoped echo protection for parameterized recorded inputs Extends ADR 0017's fill-step-scoped guarantee to the whole recording session (#1398). After #1349, a later read-only action (`wait`, `is`, `get`) can independently observe and record an app-rendered echo of an already-parameterized `fill --record-as` value in its own result or target-v1 identity evidence, re-leaking the literal even though the originating fill was protected. - SessionState gains a small, ephemeral, never-serialized literal->placeholder registry populated only from explicit `--record-as` pairs, owned by session-action-recorder.ts. - Result/event payload fields get content-aware substring redaction (reusing the fill boundary's recursive scrub) for every literal registered so far in the session, longest-literal-first. - target-v1/targets-v1 identity evidence is never silently text-substituted while still claiming a trustworthy identity (replay compares against the live tree, which re-renders the real value). A landmark-mode (wait) echo is dropped to no annotation, exactly like #1349's existing identity-empty case, so an echoing landmark can no longer serve as an ADR 0016 destination guard. Action-mode evidence (get/is/mutating actions) redacts the label and downgrades verification to "unverifiable" instead, since ADR 0012/0016 forbid dropping required identity evidence. - Amends ADR 0017 (new mechanism), ADR 0012 (#1349/writer-invariant cross-references), and ADR 0016 (destination guard cross-reference). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RarRVX34ZW25TJejBZJ2Ui * fix: placeholder-safe single-pass multi-literal redaction Addresses review feedback on #2013: sequential single-literal replacement (register somethinglong -> ${ABC}, then ABC -> ${OTHER}) could rewrite a placeholder produced by an earlier pass, corrupting it to ${${OTHER}}. Replaces the per-pair sequential loop with one placeholder-safe left-to-right multi-literal pass (parameterizeAgainstLiteralMap): it never re-scans text it has already emitted, so no literal can be matched inside another pair's placeholder token in either direction. A registered literal is matched before checking for an existing placeholder token, so a value that itself happens to look like ${SOMETHING} is still redacted correctly. The scan uses a sticky regex instead of slicing per character, and literal pairs are sorted once per payload/evidence walk instead of once per string leaf. parameterizeRecordedFillPayload/parameterizeBackendOutput are generalized to take injected leaf-transform/carries callbacks so the single-pair fill-boundary path (with its existing whitespace-collapse behavior) and the new multi-pair session-wide path share one structural traversal. Adds regression coverage for both result payloads and action-mode target evidence, plus the placeholder-shaped-literal edge case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RarRVX34ZW25TJejBZJ2Ui * fix: unexport parameterizeAgainstLiteralMap (CI: fallow dead-code gate) Only used internally within this file (by parameterizeRecordedResultEcho and parameterizeTargetEvidenceEcho); the export had no consumer outside the module, which the fallow audit correctly flags as dead code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RarRVX34ZW25TJejBZJ2Ui --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
50f460cce4 |
refactor(snapshot): establish presentation ownership boundary (#2005)
* refactor(snapshot): establish presentation ownership boundary * docs: keep context glossary within budget * fix(snapshot): address presentation boundary review * test(snapshot): ratchet eager closure budgets |
||
|
Michał Pierzchała
|
bcca714a07 | refactor: move gesture family to platform runtime (#1952) | ||
|
Michał Pierzchała
|
759f175332 |
refactor: move Wave 5 touch commands to platform runtime (#1987)
* refactor: move touch commands to platform runtime * fix: require direct selector touch binding * fix: address touch runtime review * fix: classify maestro direct click guarantee * fix: distinguish Maestro direct selector dispatch |
||
|
Michał Pierzchała
|
cc521decb8 |
test(structure): per-package eager-closure budgets — the ADR-0019 loading-shape probe (#1739) (#1965)
* test(structure): per-package eager-closure budgets (#1960) ADR-0019 requires platform-package façades to stay implementation-lazy and is explicit that a startup threshold alone is not a substitute for preserving the loading shape. #1950 built the AST-level walker (eager-import-closure.fixtures.ts) and proved the planted-red procedure on one file (session-teardown.ts's android-helper denylist); this generalizes it into a data-driven budget table so any workspace-package façade -- or a designated hub module -- can get an eager-closure ceiling without a bespoke test. Seeds a budget for every packages/*/src/facades/*.ts file (discovered the same way package-boundaries.test.ts discovers façades, not hand-listed) from its measured current closure size, plus a platform-implementation denylist for façades whose contract is implementation-neutral vocabulary. Also demonstrates the mechanism on two designated hub modules (cli.ts, session-teardown.ts) alongside their existing, more specific ad hoc pins. Closes #1960 * test(structure): derive facade roots from manifests, add edge chains, reseed tight Review findings on #1965: 1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every package that publishes its entry surface straight from the manifest — including all six `packages/platform-*/src/index.ts` façades, the exact subject of ADR-0019's implementation-laziness rule. Discovery now derives from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/` files, reusing the R11 helper rather than reimplementing it so the two gates cannot disagree about what an entry surface is. The table grows from 14 façades + 2 hubs to 95 entry surfaces + 8 hubs. 2. Budgets carried a few files of slack each. They are now exact ratchets with no headroom, matching how the repo pins R9/R10 and test-file size: growth is allowed, it just has to be a visible number change in the diff of the PR that causes it. Every budget is reseeded from post-#1969 measurement. 3. Violations printed a flat sorted set, which named the offender but not the route. `eagerClosureGraphOf` records each file's discoverer, so failures now print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps its contract and is expressed in terms of the new walk; per-file edges are memoized, which also cuts the existing pins' runtime (cli closure test 2336ms -> ~550ms). The platform-package façades evaluate exactly one module each — themselves — so their budget of 1 is the tightest statement of "metadata-eager, implementation-lazy" the walker can make. * test(structure): make the eager-closure pins exact, bounded, and single-owner Second review pass on #1965 found four holes, two of which were places the PR text claimed a property the code did not have. 1. Rows were documented as exact ratchets but asserted with `<=`, so a shrink silently became headroom a later regression could grow back into. The comparison is now equality, in a pure `classifyBudget` with a separate message for each direction ("lower its pin to N in this PR so the ratchet keeps the gain"), matching test-file-size-ratchet.ts and the R9/R10 pins. 2. An over-pin failure printed a chain per evaluated module — 361 of them for src/cli.ts. It now prints a bounded attribution: the entry's heaviest direct edges (capped at 4) with a couple of representative deep routes each, ranked so a newly added import sorts first. The comment states plainly that this attributes by shortest import route and does NOT diff against a recorded baseline; naming a true delta would mean checking in ~1,500 module paths and rewriting them on every contracts refactor. 3. Discovery reimplemented a one-level `src/facades` scan while canonical R11 discovery is recursive, so a nested façade file could be covered by R11 and silently missing here. `facadeEntryFiles` is now a single exported owner in package-boundaries.ts that both R11's façade gate and this table consume. 4. Rows were converted to Sets before any uniqueness check, so a duplicate was unobservable. The table is now two `Record<string, number>` literals keyed by path, making an in-record duplicate a TypeScript error (ts1117); the only remaining case — one path in both records — is asserted on the array. Each of the four holes gets a test that fails when the rule is broken, since a tree that happens to satisfy its pins cannot distinguish a correct rule from a vacuous one. Writing those found a real bug in the duplicate check itself (`Set.add` returns the Set, so the filter never matched). Pins reseeded on |
||
|
Michał Pierzchała
|
8d280a2081 |
refactor(request): move the device-inventory context out of core (#1993)
`device-inventory-context.ts` is an AsyncLocalStorage holder: it imports only `node:async_hooks`, kernel errors, and contracts types, and nothing from `core`. `daemon-modularity.ts` already describes `src/request/` as "request-global daemon plumbing (progress sinks, cancellation, AsyncLocalStorage)", and `request/progress.ts` is the same shape — a `withX`/accessor pair over one store. Living in `core` (rank 2) made the platform-runtime composition root reach up to rank 2 for `listLocalDeviceInventory`. Injecting the lookup would not have fixed that: the only caller, `platform-runtime-operation-host.ts`, is in the same zone, so the edge would move rather than disappear. Relocating the module does remove it, and every other consumer — daemon (rank 4) and `core/dispatch-resolve.ts` (rank 2) — now reaches down instead of sideways or up. Pure rename; the module body is unchanged and 14 import specifiers are retargeted. R17's `DEVICES_INVENTORY_IMPORT_SOURCES` pin follows the module: the rule still asserts the devices handler imports the neutral gateway, does not shadow it, and calls it. This clears the last composition-root upward value edge that was not deliberate. Confirmed with `pnpm depgraph`: those edges drop from 3 to 2, and the two that remain are the documented ones — `provider-limrun-runtime.ts` (an in-file comment marks it a deliberate static seam) and `runtime.ts -> commands/index.ts` (`bindCommands` is the aggregate binder). Cycle, back-edge and R6 counts unchanged. Claude-Session: https://claude.ai/code/session_01WWLBCBDBpdR8z1aCmDerXS Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
296447707e |
refactor: migrate back/home/orientation/tv-remote/keyboard to the request-bound device runtime (#1955)
* refactor: migrate back/home/orientation/tv-remote/keyboard to the request-bound device runtime Continues the ADR 0019 platform-runtime migration (Wave 5 generic leaves): five generic-route commands move off dispatchKnownCommand/Interactor legacy dispatch onto fact-owned admission, one bind per handler. keyboard uses the R35 action-selected single-bind pattern (status/dismiss/enter each admit and bind independently). All 8 owner runtime packages gained fact-cell tests for the new operations; six smoke-coverage integration oracles and nine daemon/capability unit test files were updated for the retired capability- catalog admission these commands no longer carry. * refactor: extract shared interactor-resolution prelude in keyboard-runtime bindKeyboardStatus/Dismiss/Enter repeated the same signal-check + resolveInteractor call; factor it into resolveKeyboardInteractor so each binder is a two-line call instead of a six-line copy. No behavior change — the three contract-module mutants planted earlier in review still kill on this shape. * fix: refuse watchOS admission for back/home/orientation/keyboard; pin tv-remote non-TV parity P1: watchOS has no constructible Apple interactor (XCUITest cannot drive its UI, ADR-0009), matching the existing captureScreenshot/captureSnapshot/readTextAtPoint/findSelector pattern in this same file. appleBackFact/appleHomeFact/appleMobileInputEligible admitted every Apple OS but tvOS/macOS, wrongly including watchOS. Facts now refuse watchOS explicitly for back, home, orientation, and keyboard dismiss/enter, with a fact-cell test asserting no binding for every one of them. P2: verified the daemon's generic-route capability gate already reproduced the retired per-platform tv-remote hint text (message stays the generic "<command> is not supported on this device", hint carries the owner-specific text) for every device that could reach dispatch in the old system -- the retired handleTvRemoteCommand's own "supported only on TV targets" check was unreachable there and only exercised by a test calling dispatchCommand directly. Added a daemon-level test pinning the exact iOS and Android-mobile hint strings to make that parity explicit instead of implicit. Also fixes a fallow complexity finding the P1 test edit introduced by splitting the fact-cell assertions into five small named helpers instead of one large function. * fix: stop orientation-runtime.test.ts's router-join test from hitting real adb Root-caused the CI-only Coverage failure (unreproducible locally in isolation, reproducible 2/2 in the full CI run): every generic-route leaf this migration touches carries `androidBlockingDialogGuard: true`, and `dispatchGenericCommand` calls `ensureNoAndroidBlockingDialogReady` unconditionally for any `platform: 'android'` session reaching the real request router -- regardless of whether admission is fact-based or capability-based. That check calls `getAndroidBlockingDialogFocus`, which shells out to the real `adb` binary. orientation-runtime.test.ts's "request router joins..." test used a synthetic `platform: 'android'` device through `createRequestHandler` (the real router), without stubbing the platform ADB layer -- only the runtime gateway was mocked. On a host with a real `adb` binary (my machine) the subprocess fails fast and `allowFailure` tolerates it, costing ~800ms-1.1s but still succeeding. On a host with no `adb` binary at all (CI's Coverage job, a plain unit-test lane with no Android SDK) the spawn itself throws, which isn't something `allowFailure` catches, producing exactly the observed `ok: false` unsupported-operation response. back/home/tv-remote's equivalent router-join tests already use Apple/Vega devices, so they never reached this path. Switched orientation's fixture to match -- Apple, since the fixture's facts/execution are fully synthetic and platform-agnostic regardless. Also: renamed the widely-shared 'emulator-5554'/'ios-simulator' device-id literals in back/orientation/tv-remote/keyboard-runtime.test.ts to file-scoped ids. Device claims for a `local-family` owner binding hit the real on-disk `require-owner` claim file (keyed only by canonical device id), and 27+ pre-existing test files already share 'emulator-5554'; this migration added three more consumers of it under a `require-owner` policy that reaches real admission, which was worth eliminating as a source of doubt even though it wasn't the actual root cause here. * refactor: extract navigation/keyboard concepts into sibling modules; test the real Android dialog-guard path packages/platform-apple/src/runtime.ts and packages/provider-limrun/src/app-log-runtime.ts grew past the repo's 500-line extraction threshold. Move the new back/home/orientation/ tv-remote/keyboard facts and bindings into packages/platform-apple/src/navigation/runtime.ts (new sibling module, matching deployment/runtime.ts's existing pattern), and the new keyboard facts/bindings for limrun into the existing packages/provider-limrun/src/interaction-operations.ts (which already held the sibling navigation logic). Also fix orientation-runtime.test.ts's router-join test: it previously swapped its device fixture from Android to Apple to dodge the real adb-backed blocking-dialog guard, which masked the Android route that was actually failing in CI. Keep the Android fixture and stub getAndroidBlockingDialogFocus instead, the same seam request-router-android-modal.test.ts already uses. * refactor: adopt granular contracts subpaths for back/home/orientation/tv-remote/keyboard Following main's #1969 (facade granularization), give each of this branch's five new contract modules their own package.json entry subpath and move every value-importer (owner runtime packages, the daemon binders, and their tests) off the wide @agent-device/contracts/platform facade onto the specific module that owns the symbol — the same convention #1969 established for the rest of the vocabulary. Keeps this migration's files out of the contracts-entry-closure gate and out of the eager-evaluation cost #1969 measured for the daemon's permanent hubs (registry.ts, dispatch.ts). * refactor: shared navigation/keyboard binder table; dedupe keyboard admission; drop restated types Addresses the review's finding 1 (seven per-owner copies of the same "fact-keyed table of interactor binders" pattern) by extracting bindAdmittedLocalInteractorOperations/bindAdmittedProviderInteractorOperations into packages/contracts/src/interactor-operation-catalog.ts. Each owner now requests the subset of back/home/setOrientation/tvRemote/keyboard{Status, Dismiss,Enter} it admits, instead of hand-writing `facts.operations.<key>.available ? bind…(resolver) : {}` per operation. Applied across all seven call sites (apple, android, harmonyos, vega, linux, webdriver, limrun) and collapsed limrun's two separate bind functions (navigation, keyboard) into one shared call. Finding 3 (resolveBoundKeyboardRuntime copy-pastes admit-then-wrap three times): extracted a local admitKeyboardAction<...> helper mirroring resolveBoundGenericRuntime's admit-then-defer shape, so the three action branches (status/dismiss/enter) share one admission path. Finding 6 (execute* helpers hand-restate a contract that can drift): back/ home/orientation/tv-remote/keyboard's execute functions are now typed off `BoundDeviceRuntime<typeof xRuntimeUse>` (derived from the actual bind-use value) instead of a hand-written `Readonly<{ operations: Readonly<{...}> }>` shape. Also fixed provider-limrun's `RuntimeOperationUnavailability | { available: true }` restating RuntimeOperationFact by hand — folded away entirely once the bind functions it typed were removed. Finding 7 (naming/placement): platform-apple/runtime.ts's misleadingly-named `captureOperations` bucket (held deployment/network/recording/find, not just capture) collapsed into one flat `operations` object now that the navigation bucket is a single function call instead of six ternaries. Exported RuntimeAdmissionRequest from runtime-admission.ts (needed by the new keyboard admission helper). Added packages/contracts/src/ interactor-operation-catalog.test.ts for the new shared binder table. pnpm typecheck, check:fallow, check:layering, and the full unit-core suite (1010 files / 7513 tests, one known contention-flake excluded) are green. * refactor: split generic-mutating command traits from the legacy dispatch pair Addresses the review's finding 5: GENERIC_MUTATING_LINUX_DEVICE_COMMAND_TRAITS bundled two orthogonal things (daemon/recording traits, and the legacy capability+dispatch pair migration strips), forcing every migrated descriptor to hand-expand the constant minus two fields plus an explanatory comment. Split into GENERIC_MUTATING_COMMAND_TRAITS (the shared daemon/recording traits) and LEGACY_LINUX_DEVICE_EXECUTION (the dispatch/capability pair). back/home/orientation/tv-remote (this migration) and focus (an earlier one, same pattern, previously a stale reference to the retired constant name) now spread the trait constant directly instead of hand-expanding it; the still-legacy `scroll` descriptor spreads both pieces, equivalent to the retired constant. pnpm typecheck, check:fallow, check:layering, and the registry/daemon test suites are green. * refactor: table-ify packages/contracts/src/keyboard-runtime.ts's three-way duplication Finding 3's second half: the three bindKeyboardX functions and six bindLocal/ProviderKeyboardXInteractor entry points differed only by method name and label string. Replaced with one generic bindKeyboardAction<Key> dispatching off the operation key (interactor[key], resolved from a small label table) plus two shared local/provider dispatch helpers the six named exports each call with their own key — collapsing three copies of the bind logic into one and six near-duplicate entry-point bodies into one line each, while keeping every exported name and type signature unchanged. pnpm typecheck, check:fallow, check:layering, and pnpm check:affected --run are green. * refactor: parameterize runSessionOrSelectorDispatch with an execute strategy Addresses the review's finding 2: handleKeyboardCommand re-implemented runSessionOrSelectorDispatch's orchestration step for step (session/selector guard, device resolve, ref-frame expiry, record) instead of reusing it, because the shared function had no seam for keyboard's bind-and-execute admission — only the legacy requireCommandSupported + dispatchCommand path. That left the shared orchestrator with one caller instead of two, and set a precedent that would fork a new copy for each of the 28 remaining session-route migrations. Gave runSessionOrSelectorDispatch an `execute` parameter: the orchestration (guard, resolve device, admit-then-execute, expire ref frame if mutating, derive and record next session) stays in one place, and callers supply their own admission/execution strategy. Extracted `legacySessionDispatchExecute` for the still-legacy capability-gate-then-dispatchCommand shape `handleTriggerAppEventCommand` (the remaining legacy caller) now passes explicitly, and `keyboardSessionExecute` for keyboard's bind-and-execute shape. Deleted the now-fully-redundant `executeBoundKeyboardCommand` — its result recording duplicated what the shared orchestrator's tail already does. pnpm typecheck, check:fallow, check:layering, the full daemon test suite (321 files / 2271 tests), and pnpm check:affected --run are green. * refactor: extract limrun facts-runtime.ts; discriminate KeyboardDismissResult by owner app-log-runtime.ts was still 589 lines after the shared-abstraction fixes; moves fact assembly (limrunAppLogFacts/limrunAppLogRecoveryFacts/limrunLifecycleFacts/deploymentOptions) to a new facts-runtime.ts and the shared device-identity predicate to device.ts, the leaf both files already depend on. app-log-runtime.ts is now 336 lines. KeyboardDismissResult was an 11-field optional bag with executeKeyboardDismiss separately re-deriving platform from the device and projecting subsets by hand. Each owner (android, apple, harmonyos) now tags its own result with a `kind` discriminant, so an owner can only ever produce its own shape, and the daemon derives the wire `platform` label from `kind` instead of guessing from the device a second time. Wire output is unchanged. * fix: expire ref frame before the mutating call, not after; extract session/selector dispatch; derive catalog operations from facts runSessionOrSelectorDispatch awaited execute(device, session) — which bundled admission and the mutating invocation together — before expiring the ref frame, so a rejecting or timed-out invocation left a stale frame active (ADR 0014 requires expiry immediately before the mutating call, with no success-only rollback). Split the execute thunk into `prepare` (admission only) + a deferred `execute` invocation, so the orchestrator can expire between them regardless of how the invocation resolves. Added a regression test proving the frame still expires when the invocation rejects. Extracted runSessionOrSelectorDispatch and its keyboard/trigger-app-event callers into a new session-selector-dispatch.ts, matching this file's own convention of one file per command-group (session.ts shrinks from 571 to well under its 500-line budget). bindAdmittedLocalInteractorOperations/bindAdmittedProviderInteractorOperations accepted both a facts object and a separately hand-maintained `operations` array naming the same keys — a second source of truth that could drift from what the facts actually admit. Removed the array; the binder now walks the fixed set of navigation operations and lets each owner's own facts decide what binds, exactly as before but with one source of truth. * style: reformat legacySessionDispatchExecute call in session-selector-dispatch.ts * refactor: derive catalog operation list from one canonical tuple; move keyboard orchestration tests NAVIGATION_INTERACTOR_OPERATIONS was declared as a plain readonly array independently of the NavigationInteractorOperation union it walked, so a future union member could compile without ever being added to the walk list, silently preventing an admitted fact from binding. Made the tuple the single canonical value: the union type is now derived from it via `(typeof TUPLE)[number]`, so LOCAL_BINDERS/PROVIDER_BINDERS' Record<NavigationInteractorOperation, ...> completeness is checked against the same tuple, not a separately hand-kept list. Added a regression test binding all seven operations at once to pin the runtime walk, independent of the type-level guarantee. Moved the four keyboard-orchestration tests (the two ADR 0014 ref-frame seam tests plus the two session/selector-guard tests) out of the mixed appstate/perf test file into a new session-selector-dispatch.test.ts, colocated with the file they exercise. Strengthened the rejection regression test to assert the frame is already expired from inside the rejecting keyboardDismiss callback itself, pinning the exact pre-invocation seam rather than only checking the end state after the dispatch settles. * fix: restore back/home/orientation/tv-remote/keyboard-runtime exports lost in rebase Rebasing onto origin/main dropped these five package.json export entries during conflict resolution (the granular-subpath commit's package.json changes silently lost during merge). Restored, confirmed by pnpm typecheck across all 17 workspace packages and the full unit-core suite (1023 files / 7581 tests). * test: pin the exact point the live iOS email field value goes missing Two prior CI runs on this PR saw the seeded email field ("ada@example") end up containing only a typed suffix (".test") by the time the flow reads it back at the end — after fill, keyboard dismiss, coordinate refocus, and type. Since this PR touches executeKeyboardDismiss's response shaping, the reviewer asked to disprove keyboard dismiss as the cause rather than assume the pre-existing dropped-keystroke flake pattern applies. Added two read-back checkpoints: right after seeding (before dismiss runs at all) and right after dismiss (before the coordinate refocus + type steps that follow). If both hold "ada@example", the loss happens during refocus/type, not dismiss — matching the documented flake, not a regression in this PR's diff. * refactor: make keyboard status/enter owner-discriminated too; trim review-round prose KeyboardStatusResult and KeyboardEnterResult were bare objects; executeKeyboardStatus and executeKeyboardEnter derived the wire platform label from device.platform via keyboardPlatformLabel, the same re-derivation already fixed for dismiss. Each owner now tags its own result with a kind (android's status/enter as 'ime-probe' and 'android-acknowledged', harmonyos's enter as 'harmonyos-acknowledged', apple's enter as 'visibility-echo'), and the daemon derives platform from a kind-keyed lookup table for all three actions. keyboardPlatformLabel and its isIosFamily import are gone — nothing derives platform from the device anymore. Android and HarmonyOS's enter acknowledgments are structurally identical (empty besides kind), so the discriminant alone — not result shape — is what tells the daemon which owner actually ran. Added a harmonyos enter test alongside the existing ios/android ones so all three owners are covered for both dismiss and enter's kind-to-platform mapping. Also trimmed several comments that narrated which PR review round motivated them down to just the durable invariant or rationale — the type shape, test names, and assertions already carry the proof. |
||
|
Michał Pierzchała
|
03c3984066 |
perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump (#1969)
* perf(contracts): granularize entry surfaces so hub importers stop evaluating the facade clump
`@agent-device/contracts/platform` unions 32 vocabulary modules and
`/interaction` another 18. A file that value-imports either evaluates the whole
union to reach one function, and because permanent hubs sat behind them —
`command-descriptor/registry.ts`, `core/capabilities.ts`,
`interactors/register-builtins.ts`, `command-descriptor/platform-execution-entry.ts` —
that union rode into roughly half the unit suite's test graphs.
Give every vocabulary module its own entry subpath and move all value-importers
onto the module that owns the symbol. Type-only importers are left alone: `import
type` is erased, so it already evaluated nothing.
Measured with the #1950 eager-import-closure walker over all 974 unit-core test
files, against base
|
||
|
Michał Pierzchała
|
d0547dcb97 |
refactor: complete the find cutover onto the request-bound runtime (#1944)
* refactor: complete the find cutover onto the request-bound runtime The deferred Wave 4 unit for #1739 (R35), unblocked by focus (R40) and type (R41). Find's read-only legs, focus leg, and type leg already ran bound; the one remaining direct platform execution was the mutating-target capture, which built createSelectorCaptureRuntime without a bound capture and fell through the legacy dispatch branch reserved for "the last one to migrate". - The mutating path now enters resolveBoundSelectorCapture — the selector family's shared admit-then-bind entry, which already named find in its intent table — and threads the bound capture into the target capture. - Find was that last one: `capture` on SelectorCaptureRuntimeParams is now required and the legacy fallback branch is deleted. The backend's `bound` becomes required-to-state, with the observation-free duration wait (`wait 400`) as the one declared absence — its runtime now carries no capture backend at all, so an accidental capture fails loudly instead of falling anywhere. - The descriptor flips to device-runtime with findRuntimePlanUses (the selector-text plans shared with get, plus focusRuntimeUse and typeTextRuntimeUse); the capability bucket and both overlay memberships (HARMONYOS_SUPPORTED_COMMANDS, WEB_QUERY_COMMANDS) are deleted. - R35 lands with the selector family's shared operation owners; the capture and backend tests move off the dispatch mock onto the bound seam, which is where the poll-deadline and private-ax-pin assertions actually live now. Wave 4 is complete: layering recognizes 26 migrated commands. * fix(find): one action-selected bind per mutating handler (ADR 0019 §9) Review P1 on #1944: a mutating find performed up to three separate facts/admit/bind projections — capture, then focus, then type re-admitted per leg. The request handler now resolves ONE action-selected plan and binds once: - New selector intents find-focus / find-type carry combined uses (capture + focusPoint, capture + focusPoint + typeText) through the same admit-then-bind path and plan machinery every selector capture uses; the new bind arms reuse the existing capture selectors, so the shared operation owners stay single. Delegated click/fill resolve targets on the plain capture pair. - The handler threads the one bind's operations to the shared executors: executeFocusPoint is extracted as the single lexical owner of the focusPoint call (R40's owner claim follows it), and executeBoundTypeText's runtime param narrows to the operations it actually uses so find can pass its own broader bind through it. - findRuntimePlanUses becomes the full action-selected set (eight uses), and the descriptor test pins each use's exact requirement list. - Regression: find focus and find type each assert exactly one facts inspection and one bindDevice call — the pre-fix handler fails both (two and three binds respectively). Live re-verified on iPhone 17 Pro at this head: find focus and find type both execute through the single bind, route synthesized-first-responder, typed text visible in the captured tree. |
||
|
Michał Pierzchała
|
81409f1a7c |
refactor: migrate type to the request-bound device runtime (#1935)
* refactor: migrate type to the request-bound device runtime
Wave 5 unit 2 for #1739 (ADR 0019), find's last blocker. `type "text"` and
`find <q> type "text"` now reach the device through one admitted, request-bound
`typeText` operation instead of the `handleTypeCommand` interactor leaf and its
dispatch-table arm.
- New `TypeTextRuntimeOperations` contract riding the same `Interactor` seam as
focus/screenshot/element-text; the operation returns the interactor's own
closed `TypeTextBackendResult`, so Apple route evidence passes through and
every other owner types blind, exactly as before. The iOS synthesized-type
commit wait (#1676) is Apple-interactor-internal and moves nowhere.
- The interaction backend's `typeText` member exists only when the `type`
handler admitted and bound a runtime — no caller can fall back to legacy
dispatch, so the command keeps exactly one execution path (R41).
- `executeBoundTypeText` reproduces the retired leaf byte-for-byte: leading-ref
rejection with the same hint, space-joined positionals, the 0-10000 delay
bound, and only textEntryRoute surviving from the owner's result. Its parse
pins moved from the dispatch-level tests into the daemon runtime test.
- Exact-owner facts replace the capability bucket (apple sim+device, android
all-but-simulator-row, harmonyos emulator+device, linux device, web device,
vega unavailable, providers wherever their interactor is reachable) and
`type` leaves HARMONYOS_SUPPORTED_COMMANDS / WEB_INTERACTION_COMMANDS.
- The Linux desktop replay types a digit on real hardware; the coverage
manifest promotes `type` contract -> live with the two-sided count pins.
- Android/webdriver facts helpers extracted (androidTouchFact, interactorCell)
to keep inspectFacts under the complexity gate.
`find` stays legacy: both of its direct execution legs now share bound
runtimes, so the atomic R35 cutover is next.
* refactor(type): single-pass daemon routing, shared binder source, owner cell tests
Review follow-ups on #1935.
- The type handler now calls the bound executor directly: the interaction-
runtime hop validated and formatted what executeBoundTypeText validates and
formats again, so it is gone — no boundTypeText backend member, no second
result rebuild. The ADR 0014 frame expiry moves to the handler.
- New contracts/interactor-operation-binding.ts: one local resolver and one
fail-closed provider resolver shared by the screenshot, focus, and type
binders — three private copies retired, provider error text preserved.
- provider-limrun/interaction-operations.ts: the interactor-backed interaction
cells move out of the app-log owner (586 -> 563 lines, below its pre-unit
size); text interaction is composed by that owner, not defined in it.
- Every owner runtime test now pins the focusPoint/typeText fact cells and
bound-operation presence for its exact kinds: apple, android (incl. the
synthetic-simulator refusal), harmonyos, linux, web, vega (refusal + hint),
webdriver (reachability-gated, incl. inactive session), limrun (live +
recovery). The webdriver unsupported-capability row documents that
interaction gates on interactor reachability, not capture declarations.
- The Linux replay assertion is now change-sensitive: type "555" then wait for
a 555 node — no calculator button carries that label, so the wait passes only
if the keystrokes landed in the display; deleting the type step turns it red.
* fix(replay): give the calculator focus before the Linux type assertion
The change-sensitive wait exposed what the review predicted: the typed digits
never landed, because `focus 100 100` clicks the DESKTOP and takes keyboard
focus away from the calculator. The old broad assertion masked exactly this.
The retries then wedged on a latent quirk: attempt-1 leaves the pointer at
(100,100), and the next attempt's `xdotool mousemove --sync` to the same point
waits for a motion event that never comes, so every retry dies at the focus
step with a 10s timeout — which is why the lane reported step 7, not the
failing wait.
New tail: `focus 100 100` (R40 evidence + survival assert), then
`click "label=1"` — a resolved press inside the window that restores keyboard
focus, proves pointer input lands in the app, and moves the pointer off
(100,100) so retries cannot trip the mousemove no-op hang — then `type "55"`
and `wait "label=155 || text=155 || value=155"`. No button is labelled 155, so
the wait passes only if the typed keystrokes reached the display.
* refactor(type): delete the fallow SDK typeText surface, drop dead surface fields
Thermo-nuclear review follow-ups (reviewed at 82fb8c2dc; the three-hop relay
it names was already deleted in
|
||
|
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
|
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
|
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
|
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
|
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
|
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
|
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
|
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
|
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
|
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
|
62001cf210 |
refactor(record): derive session recording from the publication lifecycle (#1719)
* refactor(record): derive session recording from the publication lifecycle `SessionState.recordSession` stored an answer the script-publication aggregate already contained. Every writer set both, but nothing made them agree, and #1533 was the consequence: a `--save-script` ingress re-armed the flag behind an ABORTED status, and a bare `close` published a recording the caller had been told was aborted. That fix routed every write through one rule, which made the two agree without making disagreement unrepresentable. The field remained a second source of truth, and its doc comments had to carry the invariant that a type could enforce. Remove the field and derive the answer. `isRecordingPublication` reads recording off the lifecycle: ordinary authoring records only while ARMED; a repair transaction records for its whole lifetime, terminal statuses included. That last clause is deliberately exact rather than merely safe — `armRepairStep` armed the old flag and neither `abortRepair` nor `commitRepair` ever cleared it, so narrowing it would silently stop evidence capture for a committed repair. Whether it should is a real question, and a behavior change, so it is left alone here. What this buys, beyond one less field: - `buildNextOpenSession` and `finalizeOrdinaryCloseScript` make no recording decision at all now, so no surface can arm recording without moving the lifecycle that authorizes it. - The writer's publication gate is answered entirely by the aggregate. Its separate ABORTED check is gone: a terminal authoring lifecycle is already not recording, so one question replaces two that could disagree. - The R7 ownership ratchet drops from 23 writer-owned fields / 29 owner claims to 22 / 26, and the layering manifest loses the entry whose comment documented the smell ("deliberately set on its own by paths that record without arming a publication"). Behavior-preserving: the derivation reproduces what the flag held at every transition. The test fixtures that armed `recordSession` with no publication state described a shape production stopped producing at #1478; they now carry the lifecycle that causes recording. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFW9gJqz1wEHoowkdd1nFW * test(close-script): flush queued event-log writes before removing the tmp root CI failed the Coverage lane with ENOTEMPTY removing the test's tmp root, in `afterEach` rather than in an assertion. `SessionStore.recordAction` QUEUES its event-log append (`queueEventLogWrite`) instead of writing it, and every close path in this file records an action. Nothing awaited that write, so `fs.rmSync(root, {recursive: true})` could race it: the pending append recreates `<root>/sessions/<name>/` while rmSync is walking, and the final rmdir fails ENOTEMPTY. It needs CI's parallel load to lose the race — the file passes 12/12 in isolation locally. Await `flushSessionEventLogWrites()` before removing. The hazard is latent in any test that records actions and then removes its tmp root; this fixes the file that failed rather than sweeping the pattern, which deserves its own change. Not added to the #1419 contention-retry list: that list requires a concrete spawn/wait mechanism named per entry, and this file has none. The race was a real teardown bug, not lane contention. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFW9gJqz1wEHoowkdd1nFW * docs: correct ADR 0016 on recording vs publication for repair Review caught a real overstatement. The amendment claimed evidence capture and publication authorization are "the same question asked of the same state". That holds for ordinary authoring — ARMED both records and publishes, ABORTED and PUBLISHED do neither — but not for repair: `isRecordingPublication` is true for every repair status including `committed` and `aborted`, while the writer additionally applies `isRepairArmedWriteBlocked`, refusing a committed transaction and one that is not yet committable. State it as it is: both decisions derive from the same aggregate, but they remain distinct predicates, and collapsing them would republish a committed repair or commit an incomplete prefix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PFW9gJqz1wEHoowkdd1nFW --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
1b2e786128 | refactor: move screen recording onto platform runtime (#1724) | ||
|
Michał Pierzchała
|
b15c502318 |
refactor: extract platform network runtime (#1702)
* refactor: extract platform network runtime * fix: preserve platform network recovery routes * test: guard network parser placement |
||
|
Michał Pierzchała
|
b1ed5353d1 |
refactor: extract platform log runtime (#1701)
* refactor: extract platform log runtime * fix: clear terminal app log recovery markers * fix: preserve scoped app log tooling * fix: preserve app log cancellation * fix: handle large changed coverage diffs * fix: harden Limrun runtime identity * refactor: tighten platform log runtime * fix: close app log trust gaps * fix: accept canonical session path aliases * refactor: extract durable capture kit * fix: refresh retained log marker admission * fix: rotate app logs after process relaunch |
||
|
Michał Pierzchała
|
c06bed9f77 |
refactor: extract platform device inventory runtime (#1699)
* refactor: extract platform inventory runtime * fix: preserve scoped Apple inventory tooling * fix: preserve Apple tool cancellation * refactor: tighten platform inventory boundaries |
||
|
Michał Pierzchała
|
10ff339d14 |
refactor: declare selector resolution policy as data (#1649)
* refactor: declare selector resolution policy as data (#1630) Five native consumers of "resolve a selector against the screen" each hand-declared their ambiguity contract as inline requireUnique/ disambiguateAmbiguous literals, so the repo's real policy matrix was only discoverable by reading four files. SELECTOR_RESOLUTION_POLICIES (packages/selectors) now declares one row per caller — ambiguity kind plus the structural columns (rect, occlusion, off-screen guard, promotion, poll) — and selectorResolutionKnobs turns a row into the engine knobs it stands for. Callers consume rows; zero ambiguity literals remain in src. Semantics are unchanged by construction: each row was read off its call site. The matrix names what was previously implicit — act and get text disambiguate, is/get attrs fail closed, exists/find-reads and wait take the first match, mutating find rejects candidates unless narrowed (#1625). `reject-candidates` is declaration-only and rejected by selectorResolutionKnobs at the type level, because find enforces it through its own narrowing rather than engine knobs. resolution-policy-parity.test.ts gate-tests the matrix against the callers (ADR 0011's declared-plus-gate-tested pattern): knobs must match the named ambiguity contract, every claimed structural column must appear in the caller's source, the read/wait pipelines must genuinely lack the machinery they disclaim, and no caller may reintroduce an inline literal. Verified revert-sensitive: flipping readUnique to disambiguate and faking wait's occlusion column each fail it. Out of scope, unchanged, per the issue: the Maestro engine (ADR 0015) and the open click-implicit-wait product decision. * refactor: route wait and mutating find through the policy interface (#1649 review) P1 was right: the first head declared seven rows but genuinely routed five. selector-wait.ts never imported its row (it called listSelectorChainMatches directly), findAct consumed only requireRect while its ambiguity contract stayed bespoke, and the parity test sniffed marker strings in source files — so it stayed green across exactly that gap. Asserting about the layer I had edited instead of the behavior it produces. resolveSelectorChainWithPolicy is now the one policy-driven entry: it returns a discriminated outcome (none / resolved / ambiguous) because the rows genuinely disagree about what several matches mean, which is what previously forced each caller to re-derive its contract inline. wait and find's selector branch both route through it; find additionally asserts its row still says reject-candidates rather than assuming. The parity test is rebuilt on fixture trees driven through that interface — no source sniffing. Wiring verified revert-sensitive: flipping the wait row fails the policy tests, and flipping findAct fails REAL find handler tests (ambiguous-candidate listing), which is the proof the previous version could not produce. One behavior nuance the fixture work surfaced and now pins: disambiguation declines on genuinely indistinguishable candidates (the tiebreak is evidence, not a coin flip), so an acting row surfaces ambiguity there rather than binding one silently. * fix(test): let fallow see the host-process mock helper's real consumers Rebase onto main brought #1642's host-process-mock.ts into this PR's fallow scope, where its export reports as unused. It is not: three suites consume it, but only through `(await import(...)).pinOwnProcessStartTime` inside vi.mock factories — vitest hoists those above static imports, so the dynamic form is required and fallow cannot trace it statically. Documented suppression rather than a restructure that would break the hoisting contract. Latent on main rather than introduced here: the audit gate is changed-files-only, so main sees the file in scope only from a PR whose diff contains it. * fix: keep every candidate when a policy resolves one winner (#1649 review P1) A real regression I introduced, not a test gap: routing wait through the policy interface collapsed the candidate set to the winner, and the #1349 landmark check is satisfied when SOME match carries the recorded identity. A first same-selector impostor therefore hid a later genuine landmark and timed the wait out. The resolved outcome now carries `matchedNodes` — the full candidate set of the alternative the winner came from — so a policy that picks one node no longer throws the rest away. wait passes that straight to the landmark check, restoring the original semantics. Regression test added at the within-one-poll shape the existing suite did not cover (both candidates in the SAME capture, impostor first); verified it goes red against the singleton reconstruction it replaces. * refactor: declare only the policy fields the matrix enforces (#1649 review) The occlusion / offscreenGuard / promotion / poll columns were never consumed by resolveSelectorChainWithPolicy or selectorResolutionKnobs: changing any of them left behavior and the suite green, so they were unverifiable claims that read as truth. (My earlier source-sniffing test "verified" them by grepping caller files for marker strings — which is why it also stayed green when a row was disconnected entirely.) The matrix now declares exactly what it enforces: the ambiguity contract and the rect requirement, both consumed by the resolution interface and pinned behaviorally. A new test asserts every row's field set, so an unenforceable column cannot reappear without coverage — verified by re-adding one and watching it fail. Routing the structural stages into typed behavior is tracked in #1656 with the constraint that each field must be consumed, not merely declared. * fix(selectors): flatten the policy outcome at the package boundary `PolicyResolutionOutcome.resolution` was typed as `AstSelectorResolution` and the root façade returned it unchanged, so the parser AST #1589 confined to `@agent-device/selectors/ast` came back through a nested field. `selector-wait.ts` reading `outcome.resolution.selector.raw` was the runtime proof. The existing boundary gate reads exported *names*, so it could not see this. The public outcome now lives beside `SelectorResolution` in public-resolution-types.ts with its selector as text; the parser-side shape is renamed `AstPolicyResolutionOutcome` and stays package-private, and the façade wrapper flattens on the way out — the same treatment `resolveSelectorChain` already gave `AstSelectorResolution`. Two new pins, both verified red against the shape they replace: a behavioral one asserting the façade returns selector text under every policy row, and a structural one asserting resolution shapes are re-exported from public-resolution-types.ts rather than from a parser-side module — which is what distinguishes the leak from a correct re-export in a name list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Rva4YGtSCAKJqH5PbpcCU --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
d85072d935 |
perf(cli): route command aliases through the help fast path (#1641)
* perf(cli): route command aliases through the help fast path bin.ts's `--help` fast path resolved aliases through a hand-written two-entry table that had drifted out of sync with the real CLI_COMMAND_ALIASES registry (five entries). `tap`, `launch`, and `relaunch` missed the table and silently fell through to a full runCli() bootstrap just to print static help text (~150-165ms vs ~45-50ms for aliases already in the table). Delegate to the shared normalizeCliCommandAlias registry instead of the stale local table, so every alias the registry knows about gets the fast path automatically. * test(cli): add R12 layering guard for bin.ts's alias delegation The unit test added for the alias fast-path fix (cli-help-alias-fast-path.test.ts) calls normalizeCliCommandAlias directly, so it stays green even if bin.ts itself reverts to a hand-rolled table — it pins the registry composition, not bin.ts's own wiring, and bin.ts cannot be safely unit-imported (it runs unguarded top-level dispatch on import and is deliberately excluded from coverage). Add an AST-based structural guard instead, in the style already established by scripts/layering/session-state.ts, facade-exports.ts, and zero-dep-jobs.ts (oxc-parser's module/program records, not a line scan, so a fixture's string literal can't produce a false hit). R12 asserts two facts about src/bin.ts: it holds a value import of normalizeCliCommandAlias from commands/cli-command-aliases.ts, and it contains none of the registry's own alias tokens as string literals. The token list is read out of the registry's own source (CLI_COMMAND_ALIASES's `alias:` property values), not hard-coded, so a future sixth alias is covered automatically. Both facts were false on the pre-fix bin.ts, verified by reverting locally and capturing the failure before restoring the fix. Wired into the existing check:layering chain (already part of check:tooling), next to R7's session-state ownership rule, which pins the same "delegate to your single owner" shape. * test(cli): pin the alias-resolver call into buildCommandUsageText (R12 P2) Maintainer review of R12 (PR #1641): import-presence and literal-absence alone let bin.ts regress to buildCommandUsageText(helpTarget) while the normalizeCliCommandAlias import stays in place, used harmlessly elsewhere (or not at all) — the real-tree gate stayed green through that exact regression. Add a third fact: bin.ts's call to buildCommandUsageText must receive, as its argument, a call to the LOCAL binding the resolver was imported as (aliasResolverLocalName + usageTextCallsResolver, both AST-based). Binding by local name rather than the literal export name means a renamed import (`as resolveAlias`) still verifies, and an unrelated same-named local cannot be mistaken for it. Verified by reverting locally to exactly the missed regression — import left in place, call reverted to buildCommandUsageText(helpTarget) — and confirming R12 now fails where the two-fact version passed; restored after. Two negative fixtures pin the scenario going forward: import present but unused, and import present but used only unrelated to the call. * test(cli): make R12's delegation fact universal and value-bound The previous fact 3 asked whether *any* `buildCommandUsageText(resolver(...))` existed in bin.ts. That quantifier is satisfied by a decoy call while the line that actually ships resolves nothing: void buildCommandUsageText(normalizeCliCommandAlias('open')); const commandHelp = buildCommandUsageText(helpTarget); Fact 3 now requires EVERY `buildCommandUsageText` call to receive the imported resolver applied to the fast path's own help-target binding, which rejects both lines above independently. The help-target name is read from bin.ts (the variable initialized by `resolveSimpleHelpTarget`), so renaming it re-points the guard instead of disarming it. Because fact 3 claims binding identity by name, it also now rejects a local shadow of the resolver and an ambiguous second help-target declaration — a same-named local would otherwise let the composition read as delegation while calling something that resolves nothing. The predicate returns the reason rather than a boolean, so the gate names which of the several distinct failures happened. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Rva4YGtSCAKJqH5PbpcCU --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
d5f11f6e2f |
refactor: import package types directly — no internal re-export laundering (#1640)
* refactor: import package types directly instead of re-exporting from internal modules
Post-#1636 review feedback: internal src modules were re-exporting package
types (export type { X } from '@agent-device/...'), giving one declaration
several import paths and hiding its provenance. New rule applied repo-wide:
internal modules import directly from the owning package; only published
entry surfaces (src/sdk/* entries, client-types, finders, metro composition,
remote-config-schema) may re-export.
Eleven internal re-exports removed and ~110 import sites redirected to the
packages, the big two being CommandFlags (core/dispatch chain, 39 sites) and
SessionAction (daemon/types.ts, 19 sites). Two were already dead
(RefFrameEffect via daemon-command-registry, DiffSnapshotCommandResult via
capture/runtime/snapshot). Entry-surface chains now re-export from the
package rather than laundering through a second internal module
(client-types/client-metro MetroBridgeScope).
Side effect: the R9 type cycle shrinks again, 49 -> 47 (daemon-server
19 -> 17); ceilings lowered to match.
* refactor: drop command-schema's CliFlags re-export (#1640 review P2)
The one consumer (cli/parser/args.ts, a multi-line import the sweep's
single-line scan missed) now imports CliFlags from contracts/command;
FlagDefinition/FlagKey stay — they are src-declared types, not package
laundering.
|
||
|
Michał Pierzchała
|
d5f99bab1c |
refactor: sink backend.ts's cycle-closing types below both zones (#1632) (#1636)
backend.ts imported RepeatedInput up from commands/command-input.ts and ScreenshotResultData up from utils/screenshot-result.ts — the interface hub typed in terms of the zones that depend on it, R6's textbook inversion shape. - RepeatedInput now lives in @agent-device/contracts/interaction; command-input.ts re-exports it for its existing importers. - ScreenshotResultData already had a byte-identical canonical declaration in contracts/snapshot-types.ts (exported via contracts/capture); the utils copy is now a re-export of it, deleting the duplicate outright. Measured member-by-member: the R9 type cycle collapses 76 -> 49 files. backend.ts, runtime-contract.ts, commands/runtime-types.ts, and commands/runtime-common.ts all leave the component (27 files stranded out at once); zone ceilings lowered to the measured values (commands 33 -> 14, platforms 7 -> 2, root 5 -> 3, daemon-server 20 -> 19) and CONTEXT.md's hub list recomputed (core/dispatch.ts 8, command-catalog.ts 7, resolution.ts 6, command-descriptor/registry.ts 6). No TYPE_INVERSION_BASELINE additions. |
||
|
Michał Pierzchała
|
d919876cb0 |
refactor(daemon): one interface for the deferred interaction outcome (#1633)
* refactor(daemon): one interface for the deferred interaction outcome (#1629) The machinery answering "did that mutation actually take effect?" was three modules coordinated only through raw SessionState fields: two independent marking sites (finalizeTouchInteraction vs dispatchGenericCommand, plus a third in session-open), a resolve side buried as private functions in snapshot-capture.ts with no direct tests, freshness heuristics split across the seam, and two raw reads of session.postGestureStabilization outside the owning module. - src/daemon/deferred-interaction-outcome.ts is now the one interface: markDeferredInteractionOutcome (every mutating route, one ordering) and resolveDeferredInteractionOutcome (every snapshot capture, parameterized over the capture primitive so it is directly testable). - getAndroidFreshnessReason moves beside its state machine in android-snapshot-freshness.ts, with the module's first direct test file. - isPostGestureStabilizationPending replaces the raw field reads in direct-ios-selector.ts and selector-capture-runtime.ts. - snapshot-capture.ts shrinks 617 -> 359 lines and keeps only capture, state building, and scope resolution. - No behavior change. The R9 type cycle drops 76 -> 74 files; zone ceiling lowered accordingly. CONTEXT.md gains the "deferred interaction outcome" term. * style: format android-snapshot-freshness.test.ts * fix(layering): record the honest R9 delta — the new module joins the cycle (+1 node) The earlier 76 -> 74 measurement was an artifact: the layering scan reads tracked files, and deferred-interaction-outcome.ts was still untracked. The real delta vs main is 76 -> 77 / daemon-server 20 -> 21: the choke point sits inline on value paths that already ran member-to-member, so the cycle gains one node and zero new edges. Ceiling raised explicitly with the rationale at the baseline, per the R9 rule's own escape hatch. * refactor(daemon): host the deferred-outcome seam in the stabilization owner (#1633 review) Zero R9 growth, per review: a NEW aggregator file cannot stay out of the cycle by shedding type imports — its value imports of the two member owners close the loop regardless. The unique zero-growth host is an existing cycle node, and the stabilization owner is the only legal one (the policy module cannot value-import stabilization back, R4; the freshness module would join as a new member). So deferred-interaction-outcome.ts absorbs the post-gesture-stabilization implementation and becomes the R7 owner of postGestureStabilization: the seam lives in a node that was already on the member-to-member paths it concentrates. - markPostGestureStabilization is now module-private behind markDeferredInteractionOutcome; stabilization marking tests drive the public interface. - The freshness retry loop moves beside its classifier in android-snapshot-freshness.ts; gesture-no-effect helpers move to a leaf file. Both stay outside the cycle. - Ratchet reverted to main's exact values (76 files, daemon-server 20) — measured member-by-member vs main: the diff is empty. * refactor(daemon): extract the pure stability loop into a leaf (#1633 review) The deferred-interaction-outcome owner keeps the seam, the pending-record ownership, and the R7 clear; the quiet-window polling loop and the baseline-distrust verdict move to post-gesture-stability.ts, parameterized by hooks (capture, surface reader, the three signature comparators) so the leaf imports no cycle owners and no SessionState — verified outside the R9 cycle, which stays at main's exact 76/20. Owner drops 539 -> 374 lines. The no-effect corroboration keeps comparing against the ORIGINAL pre-gesture baseline (never a mid-loop rebased one), now stated in the leaf's doc. The stabilization loop suite drives the unchanged public adapter; the verdict suite wires the real classifier through the leaf's hook. |
||
|
Michał Pierzchała
|
d8b309c6db |
refactor(contracts): name façade exports explicitly and retire the pin table (#1614)
* refactor(contracts): name façade exports explicitly and retire the pin table Thirteen of the fourteen `@agent-device/contracts` façades were bare `export *` barrels. `facades/snapshot.ts`, added by #1582, was the one exception — explicit named re-exports — and that is now the rule. Everything #1574 built to cope with `export *` goes with them: scripts/layering/facade-symbols.ts -980 (816 pinned names) scripts/layering/facade-exports.ts -192 (readFacadeExports) scripts/layering/facade-exports.test.ts -234 (star semantics) scripts/layering/package-boundaries.test.ts -55 `readFacadeExports` re-implemented ESM `GetExportedNames`/`ResolveExport` — star-chain resolution, ambiguity rejection, diamond binding identity, cycle guards, spec-accurate `default` filtering at the star rather than the source. All of it existed to enumerate what `export *` hides. 523 of the 816 pinned names belonged to contracts, i.e. to those thirteen files. Once a façade names its exports, the façade file IS the pin, and it is visible in the diff of the file that widened rather than in a separate table a reviewer has to cross-check. `readNamedExports` (20 lines) stays and is enough: it already throws on bare `export *` and on `export default`. The pin is replaced by one structural gate — no façade may contain a bare star — which reuses that rejection rather than adding a regex. Surface equivalence verified independently, not asserted: main's own `readFacadeExports` run over the new façades, compared against main's own `FACADE_SYMBOLS` table — 31 subpaths, 0 added, 0 removed. Red evidence for the new gate: planting `export * from '../request-progress.ts'` back into facades/progress.ts fails it with the file named and the reason quoted; 12 pass / 0 fail once reverted. Not included: the `lowerAndroidTouchPlan` tuple-assertion drive-by. It needs `sampleGestureOffsets` to carry a min-arity tuple through `.map()`, which TypeScript will not infer without a typed helper — a real change to the gesture-plan contract rather than a drive-by, so it stays out. * test(layering): assert façades stay exhaustive over their sources Review on #1614 caught this conversion silently narrowing the public surface. The explicit lists were generated against the surface at fork time; #1567 landed 13 exports meanwhile — `DragOptions`, the drag-gesture vocabulary (`COORDINATE_GESTURE_KINDS`, `CoordinateGesturePayload`, the three `DEFAULT_DRAG_*` constants, `DragGestureInput`, `DragGesturePayload`, `GestureCommandInput`, `buildDragGesturePlan`, `dragGesturePayloadFromPositionals`, `normalizeGestureCommandInput`) and `MultiTargetAnnotationV1`. The `export *` barrels had been forwarding all 13 automatically; the rebase dropped every one, and only a human diff caught it. The star-rejection gate could not: it only proves a façade does not WIDEN invisibly. Narrowing is the failure an explicit list newly makes possible, because `export *` could not narrow by construction. So the property the stars gave for free is now asserted directly — every name a re-exported source declares must appear in the façade. Scoped to `packages/*/src/facades/`, the barrels this PR converted. A hand-curated package `index.ts` is a different thing: `ad-replay` deliberately publishes two values out of a much larger `internal/`, and forcing exhaustiveness there would widen a surface its owner narrowed on purpose (#1555). A source that itself carries a bare `export *` is skipped — unknowable from that file alone, and reachable because the façade re-exports the starred module directly too, which IS checked. Red evidence: dropping `MultiTargetAnnotationV1` from facades/replay.ts — one of the 13 the old gate was blind to — fails with the file, the source and the symbol named. 13 pass / 0 fail once restored. * fix(layering): close the exhaustiveness gate's starred-source hole Two review findings, plus a third the gate caught on itself. P1 — the three `DEFAULT_DRAG_*` constants join the existing public-façade suppression, alongside `COORDINATE_GESTURE_KINDS` and `normalizePublicGesture` which the same conversion surfaced. All five are #1567's drag vocabulary, made individually visible to `--production` analysis for the first time because a bare star used to hide them from that exact check. Kept rather than narrowed, for the reason the existing entry already states: the façade's surface stays byte-identical to what the retired pin table asserted, and narrowing is a follow-up with its own review. P2 — the exhaustiveness gate skipped any source carrying a bare `export *`, which dropped that module's DIRECT exports from the check too. `gesture-plan.ts` stars `gesture-plan-types.ts`, so removing `buildDragGesturePlan` from the façade narrowed the public surface and still passed. `readDirectNamedExports` now reads exactly the names a module declares or re-exports BY NAME and ignores the star, so direct exports are checked while the starred set stays covered by the façade's own direct re-export of that module. Red evidence: removing `buildDragGesturePlan` from facades/interaction.ts now fails naming file, source and symbol; 13 pass / 0 fail restored. Third, and the reason the gate is worth having: rebasing onto main after #1612 merged silently dropped `TEXT_ENTRY_ROUTES`, `TextEntryRoute` and `TypeTextBackendResult` from the interaction façade — the same narrowing class as the #1567 one review caught by hand, one merge later. The gate failed on it before CI did. Restored. |
||
|
Michał Pierzchała
|
ee473b6adc |
refactor(daemon): give the Maestro fallback and ambiguous-match details real types (#1612)
Three places smuggled structured data through untyped bags and re-read it
with runtime guards. Each gets an explicit typed boundary.
A. The resolution-suppression rule was encoded twice in
interaction-touch-response.ts — a spread ternary in the runner-payload
branch and an unconditional destructure used conditionally in the runtime
branch, with the ADR 0012 rationale living on only one source variant.
Both branches now read one `suppressesResolutionDisclosure(source)`
predicate through one `applyResolutionDisclosurePolicy` helper, where the
reason is stated once. The union field is renamed
`maestroCoordinateFallbackDispatched` (the dispatch path that ran) and
hoisted into a shared base. handleFillCommand's two-arm interactor.fill
call collapses to one.
B. `Interactor.type` narrows from `Record<string, unknown> | void` to
`TypeTextBackendResult | void`; the Apple runner boundary is the single
place the wire payload becomes that type. `maestroFallbackDetails` returns
a typed `{ used, extra }` instead of a bag both call sites re-read.
C. `details.candidates` meant two incompatible things. The device-domain
resolvers now key their list `devices`, so the shared renderer drops its
shape-disambiguation guards and the device list actually renders.
|
||
|
Thiago Brezinski
|
a13a6832ee |
feat: add selector-targeted drag gestures (#1567)
* feat: add selector-targeted drag gestures * fix: address drag gesture review feedback * fix: satisfy drag review quality gates * fix(android): lower drag trajectories piecewise * test(replay): validate drag fixture selectors * fix(ios): ignore full-viewport chrome containers * test(drag): prove destination on live devices |