mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
docs/improvement-audit-tracker
39 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
|
52ac5da091 |
fix(daemon): stop branch-named daemon before replacement (#2015)
* fix(daemon): recognize branch-named daemon entries * fix(daemon): require process identity for takeover * fix(daemon): preserve shutdown cleanup identity |
||
|
Michał Pierzchała
|
608bf7aa47 |
Harden the MCP surface: registry rug-pull fix, operator-only credentials/endpoints, device-shell argv gate, declared timeouts (#2023)
* chore(release): keep the version on main distinct from every published version
Registry scanners diff the repository's tool surface per version string, so a
released number left on main while main keeps changing is indistinguishable
from a republished ("rug-pull") version — two scans of the same version see
two different tool sets (AS-012).
- release:publish now runs release:mark-dev after npm publish, moving
package.json and the synchronized server.json to the next patch with a
-dev prerelease marker.
- release:prepare refuses to publish while the -dev marker is in place, so
a forgotten version bump cannot ship a prerelease as latest.
- Mark the current tree 0.20.11-dev: main had been sitting on the published
0.20.10 while the tool surface kept changing, which is the live finding.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8
* fix(mcp): remove credential inputs from the model-writable tool surface
Every MCP tool advertised daemonAuthToken (and the Metro tools bearerToken)
as a free-form string the model writes. The model both reads untrusted app UI
text and picks tool arguments, so on-screen text steering it to set a token
was a prompt-injection exfiltration path. Credentials are operator-owned:
- the keys are omitted from every advertised tool schema (MCP and AI SDK,
which share listCommandTools()),
- an explicit value is refused with env-var guidance instead of being
forwarded (the retired-field posture: refuse, never silently drop),
- operator-sourced values are untouched — env/config defaults still merge,
and the daemon and Metro clients keep their AGENT_DEVICE_DAEMON_AUTH_TOKEN
/ AGENT_DEVICE_METRO_BEARER_TOKEN fallbacks. CLI flags are unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8
* fix(mcp): move operator endpoints and paths off the model-writable surface; declare timeouts
Follow-up to the credential removal: daemonBaseUrl and the Metro
proxyBaseUrl are the endpoints the env-resolved tokens are SENT to, so a
model-writable value redirects the operator's token to an arbitrary server —
same exfiltration path, one step removed. stateDir, cwd,
iosSimulatorDeviceSet, and the three iosXctest* paths select operator
infrastructure, never per-call work. All of them leave the advertised
MCP/AI-SDK tool schemas and are refused as explicit input with env/config
guidance; operator env/config defaults keep flowing exactly as before
(config-backed defaults still merge, and explicit input can no longer
override them). Dropping these shared properties also cuts tools/list
substantially.
Every tool description now also declares its enforced client timeout
envelope (90s default, 180s install, unbounded only for the streaming test
runner), sourced from the descriptor registry's timeout policy so the
declared number cannot drift from the enforced one (answers AS-011, which
read the undeclared envelope as "no timeout").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8
* feat(ci): inventory every dynamic value reaching a device shell
adb shell, adb exec-out, and hdc shell join their argv into one string the
device's sh evaluates, so any unquoted dynamic element is a potential argv
injection — the class of bug the audit found (and fixed) on input text and
cmd clipboard set text. Nothing enumerated the surface, so a new call site
could regress it silently.
scripts/shell-argv is an AST-based gate (oxc-parser, same as di-seams and
layering) keeping an exact inventory of every dynamic device-shell argv
element, keyed by (file, expression) with counts: 121 values today. A new
or grown entry fails CI until the author quotes it through shellQuoteIfNeeded
or records it with --update in the same PR, making "a new value now reaches
the device shell" a reviewable diff; a stale entry fails the other way so
the inventory always matches the code. Wired as the shell-argv gate in the
lint lane and registered in the check catalog.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8
* fix(shell-argv): satisfy the fallow audit without suppressions
The Compatibility & Provenance lane's fallow audit flagged the new gate:
main was an unused export (only the self-run guard consumed it) and four
functions sat over the complexity thresholds. Restructure instead of
suppressing: the AST walk dispatches through a composite-child-field table,
the argv detection is hoisted out of the visitor, drift reporting moves into
helpers, and main is no longer exported. Behavior is unchanged — the model
tests pass as written and the regenerated inventory is byte-identical
(121 values).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8
* fix(test): close the port-reuse race in the unreachable-takeover test
Coverage (1) failed once in CI with the takeover notice missing while the
response still came from the fresh daemon — the exact signature of the
fresh fixture being handed the just-freed ephemeral port: the recorded
daemon becomes reachable and reusable (same version and signature), so the
takeover path is skipped. Bind the fresh fixture before acquiring and
freeing the unreachable port; with no bind after the close, the port can
never be reclaimed. Line-neutral so the size-ratchet pin holds.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8
* fix(mcp): enforce the advertised tool schema at a real admission boundary
P1 (reported by the PR author): hiding operator keys from tools/list did not
stop them reaching the command route. The router forwards raw tools/call
arguments verbatim and resolveMcpConfigDefaults reads them as CLI flags, so an
unadvertised `config`/`remoteConfig` key loaded an arbitrary file whose
daemonBaseUrl/daemonAuthToken then flowed to runCommand — a model-writable
redirect to an attacker endpoint with the operator's token. Reproduced:
{config: <path>} on `snapshot` put both values into the command input.
Replace the per-key operator refusal with a deny-by-default admission boundary
in the shared executor (the one path both the MCP router and the AI SDK adapter
use): every raw input key must appear in the tool's advertised schema, else it
is rejected with guidance BEFORE config/env resolution. This closes the config
loaders, the operator keys, and any unknown key at once, and makes the
advertised additionalProperties:false contract actually enforced. Operator
env/config defaults still resolve — they never arrive as tool input.
Retired keys (maxSize) are admitted so the command's own reader still answers
with migration guidance; they're exposed as metadata.retiredInputKeys for that.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8
* revert(ci): remove the check:shell-argv inventory gate
The PR author correctly flagged that this gate is an inventory, not a
security invariant: --update lets any site self-approve a raw value, and the
literal-first array heuristic is blind to indirect argv (a variable-built
subcommand, or an argv assembled in a helper). Reproduced: adb(['shell',
'input','text',text]) is inventoried, but const s='shell';
adb([s,'input','text',text]) yields no finding. Shipping it security-framed
gives false assurance.
Remove it. The sound fix — a typed device-shell execution boundary where a
raw string cannot reach adb/hdc shell without being quoted or explicitly
marked — is a ~188-site cross-package migration on device execution paths,
scoped to a dedicated follow-up PR. The two known-dangerous sites (input
text, cmd clipboard set text) already quote through shellQuoteIfNeeded on
main, so no regression. This keeps the PR focused on the MCP tool surface.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wb8EuoySL26UsdtCtRj5X8
---------
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
|
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
|
957a6727f8 |
fix(android): publish covered state from exact order evidence (#1981)
* fix(android): unify snapshot occlusion across API levels * fix(android): preserve exact occlusion evidence * fix(android): restore collective occlusion coverage * fix(android): preserve snapshot evidence across consumers |
||
|
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
|
6dd7d406c0 |
perf(android): keep the automation helper warm across fill and scroll (#1974)
* perf(android): keep the automation helper warm across fill and scroll Android permits one UiAutomation owner, so a command-scoped helper call stops the instrumentation session when it finishes and the next call pays a fresh `am instrument` start plus the UiAutomation connect wait. `fill` reads the live hierarchy four times per attempt (the pre-action target read plus the 0/150/350 ms settling samples) and every one of those reads was command-scoped, so a single fill ran four instrumentation lifecycles — seven on retry — and left `scroll` without a session for its viewport read. Thread the existing `daemon-session` scope, which app-backed sessions already select for snapshots and whose release session teardown already owns, through fill's captures and the gesture viewport read. The viewport read may now warm the session so it and the gesture that follows share one instrumentation instead of starting two. Snapshot capture and the viewport read build their capture options through one path, because the session identity is derived from them: two builders would restart each other's session instead of sharing it. Session teardown also stops force-stopping the runtime once the helper both acknowledged `quit` and its process exit was observed — that pair is the release evidence. Forced, timed-out, and aborted teardowns still force-stop, so a daemon that dies without sending `quit` cannot leave the helper squatting UiAutomation. The settling samples still capture separately; only the session is shared. * fix(android): reset the helper runtime on content-failure retirement REVIEW.md F1. Gating the teardown force-stop on confirmed release also changed what `stopAndroidSnapshotHelperSession` returning `true` means to its callers. `retireAndroidSnapshotHelperAfterContentFailure` reads that return as "the session stop was the runtime reset" and skips `resetAndroidSnapshotHelperRuntime` when it is true, so a helper whose output failed content validation three times over a daemon session was retired with no `am force-stop` at all, while the one-shot arm of the same recovery still reset the runtime. The session stop now takes `resetRuntime`, and the retirement passes it. Content failure is a recovery path, not a clean release: the helper answered with output we could not trust, so the next capture must meet a runtime that was reset, and "it quit politely" is not a reason to leave a suspect process owning it. Genuine session close still skips the round trip, which is the optimization this branch exists for. The pre-existing test "content failure retirement does not layer a second reset over a persistent session stop" rested on a premise this branch had already made false — stopping a live session was no longer the runtime reset. It is renamed to "makes the session stop reset the runtime instead of layering a second one" and asserts the requirement it now depends on, so its title states what it enforces. The reviewer's probe returns as a regression test in `snapshot.test.ts`. REVIEW.md F2. `observeAndroidSnapshotHelperProcessExit` returns an observation that knows whether the end it saw is release evidence: the process must have been alive when the teardown started watching and must then exit with code 0 and no terminating signal. A signal, a non-zero code, and a host child that was already gone before `quit` was sent all mean the transport died — and in the pre-died case the acknowledgement is positive evidence that the device-side helper OUTLIVED its host through the open forward. None of them confirm release now. What is left is declared at the gate: host-side exit codes are only as strong as adb's exit forwarding, and a device without shell protocol v2 can report 0 for an instrumentation that did not finish. REVIEW.md F3. `&& graceful.exited` survived deletion against all 423 tests. Two named tests now fail without it, both cheap because the stricter evidence above makes acknowledged-but-not-released reachable without waiting out the 11 s graceful-exit timeout. Gates: `pnpm vitest run src/platforms/android` (44 files, 423 tests) and with `src/core/interactors` (426 tests), `pnpm typecheck`, `pnpm lint`, `pnpm format`, `pnpm check:fallow --base origin/main` (24 changed files, clean). No emulator is attached in this worktree, so the teardown-path force-stop counts were not re-observed live. * test: lower the snapshot.test.ts ratchet pin to 1,495 (PR #1974 CI) This branch extracted the helper-session, retirement, and touch-helper cases out of src/platforms/android/__tests__/snapshot.test.ts into sibling files, taking it from 1,658 to 1,495 lines. The test-file size ratchet fails an un-banked shrink ("lower its pin in this PR so the ratchet keeps the gain"), so bank it. The pin only moves down; nothing grew into it. * refactor(android): own helper-session lifecycle and text input in their own modules PR #1974 review, blocker 1. Both files the branch grew were already at the extraction threshold, so the behavior landed on top of a boundary that should have moved first. `snapshot-helper-session.ts` (563) carried two concepts: who owns the device's UiAutomation, and what runs over that ownership. Ownership — the live-session registry, the enable gate, session identity, start, reuse, and retirement — moves to `snapshot-helper-session-lifecycle.ts`. What is left is 170 lines of snapshot capture and touch commands that acquire through it and never reach the registry themselves; the touch path reads ownership through `getLiveAndroidSnapshotHelperSession` instead of the map. The stale re-export block for the retirement symbols is gone with its last importer. `input-actions.ts` (508, pre-branch 501) sheds every text-entry path to `text-input.ts`: provider injection, the test IME, the adb-shell writer, and the fill orchestration this branch changed. What is left is 172 lines of pointer, key, and gesture actions — below the pre-branch length. Tests follow their owners. `snapshot-helper-session.test.ts` splits around the same seam into a lifecycle file (start, reuse, identity, teardown, quarantine) and a capture file (budgets, cancellation, fallback), and its session fake becomes a named export of the sibling fixture module, which drops the duplicated process double. `input-actions-fill.test.ts` and `input-actions-test-ime.test.ts` are renamed to `text-input-*`, and the `typeAndroid`/`fillAndroid` cases in `input-actions.test.ts` move to a new `text-input.test.ts`. Blocker 2. The teardown skipped `am force-stop` on an acknowledged quit plus a clean host exit, and its own comment admitted the hole: adb without shell protocol v2 exits 0 whenever the connection closed cleanly, including for instrumentation that never finished, so that exit status was never proof the device released UiAutomation. A residual-risk paragraph is not an invariant. `adb-shell-protocol.ts` asks the transport instead. `adb features` lists only the features both ends negotiated, so `shell_v2` there is positive proof that a host `adb shell` child's exit status is the device command's. The skip now requires it; an unsupported transport, a failed probe, and an adb too old to know `features` all keep the device-side stop. The probe is host-side and cached per device, so it costs one query per device rather than one per teardown, and it clears with the session registry. Quarantine keys on the same proven-release signal, so an unproven quit whose stop also failed now reports unknown ownership instead of trusting the quit. Red first: a transport without `shell_v2` and an unanswerable probe both skipped the stop against the pre-fix gate, and the cache fence saw no probe at all. Rebased onto origin/main, which had moved the fill files. |
||
|
Michał Pierzchała
|
443bbd0cb8 |
fix(mutation): move the repo size ratchet where the lane cannot reach it (#1977)
Stryker runs the suite from a sandbox copy under `.tmp/stryker/`, so a test that asserts about the repository checkout itself — its files on disk, or its git history — reads a repository that does not exist. `test-file-size-ratchet.test.ts` is such a gate, and it fails there for two independent reasons: `disableTypeChecks` (Stryker's default) prepends `// @ts-nocheck` to every copied file, so all 26 pinned files read one line longer than they are; and the sandbox has no `origin/main`, so the gate's history-backed half cannot resolve its merge-base. Fixing either leaves the other. Its own `.tmp` skip entry cannot help: that is matched relative to `REPO_ROOT`, which inside the sandbox *is* the sandbox. Move it to `scripts/__tests__/` and include it explicitly in `unit-core`, the address the repo already uses for maintained gates that are not `src` tests. `KERNEL_TEST_FILE_RE` admits only root/package `src` tests, and its comment already names `scripts/__tests__` as unreachable by construction — so the gate leaves every mutation lane by virtue of where it lives, with no classifier to recognise it and nothing to keep in sync. This replaces the source-text scanner of the previous revision, which was the wrong boundary: it sniffed for a single-quoted `walk-files` import or the string `origin/main`, so a behavioral test could match and be silently excluded while an equivalent repo gate using double quotes, another walker, or another base ref would be missed. The scanner, its test, and its justifying comment are all gone. `REPO_ROOT` and the walked roots are unchanged — both addresses are two levels below the repo root, and `TEST_ROOTS` already included `scripts`, so the gate measures exactly what it did before. The one new assertion pins the invariant this now depends on: `isKernelTestFile` accepts root/package `src` tests and rejects `scripts/__tests__`. Widening that pattern would silently pull the gate back into every lane. Verified with `pnpm mutation:run --modules kernel-errors`: scope 804 -> 803 test files, dry run clean, lane `pass` at stage complete, score 74.8% (187 killed / 63 survived / 250) — unchanged. `pnpm mutation:test` 39/39, `pnpm check:layering` 181/181, `typecheck`, `lint`, `format` clean. `stryker.config.json` is untouched, so scores stay comparable. Unblocks #1964, whose two mutation checks fail on main's tip without its code. |
||
|
Michał Pierzchała
|
cb65d6ca1f |
refactor(tests): replace the test-utils barrel with direct module imports (#1956)
* refactor(tests): replace the test-utils barrel with direct module imports The barrel re-exported 13 modules, so every importer evaluated all of them (store-factory alone drags 16 daemon session-store files; property-arbitraries drags fast-check). Importing the backing modules directly cuts the unit suite's aggregate eager module evaluations from 153,401 to 144,344 (-5.9%), measured with the eager-import-closure walker. Deleting the barrel makes the tax unrepresentable instead of pinning it with a guard test. * docs(testing): point fixture guidance at the test-utils modules, not the deleted barrel * test: extract replay session fixture |
||
|
Michał Pierzchała
|
5676d5ff8a |
refactor(apple): drop dead runner code and collapse duplicated helpers (#1936)
* refactor(apple): drop dead runner code and collapse duplicated helpers Removes declarations with no consumers (findScopeElement, interactiveTypes, two unused PresentedNode convenience inits) and collapses copy-pasted logic: DataPayload now relies on the synthesized memberwise init, TvRemoteButton is String-raw-valued, point-hit sorting shares smallestElementFirst, command-id trim-or-nil lives once on RunnerCommandJournal, scroll/desktopScroll share direction and durationMs validators, and the seven inline NSError refusals use unsupportedOperationError. elementTypeName reads a table pinned by the visibility-fold parity test. The packager now skips files whose unit-test blocks were their whole body, so 10 test-only files stop shipping (and stop compiling on user machines) as empty translation units. Packaged Swift: 432.3 kB -> 427.8 kB; 56 files instead of 66. Net -144 lines. * test(apple): pin skeleton-file exclusion in the packaging guard The strip fixture always kept runtime content, so reverting the skeleton-skip branch left every guard green. The new fixture's whole body is unit-test blocks; the packaged path must be absent while a non-skeleton sibling still ships. Observed red with the skip branch disabled before re-enabling it. * refactor(apple): tighten runner cleanup boundaries |
||
|
Michał Pierzchała
|
30de1597d3 |
ci: attribute native package size and trim Apple runner (#1934)
* ci: attribute npm package size by shipped component * refactor: modularize size reporting and trim Apple runner * ci: preserve size reporter modules across base checkout |
||
|
Michał Pierzchała
|
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
|
250e30a578 |
test(bench): falsification fixtures for oracles + typed runner outcomes (#1893)
* test(bench): falsification fixtures for oracles + typed runner outcomes
Two deterministic PR-time quality gates for the help-conformance bench
(the repo's single non-gating small-model planning oracle):
- Every EXPECTATION_SCORERS entry in help-conformance-case-checks.mjs
now has a falsification fixture (a minimal passing witness plus at
least one known-bad counterexample, and a metamorphic variant where
useful) in the new help-conformance-expectation-fixtures.ts, run
through the real validatePlanCommands/scoreExpectations pipeline.
help-conformance-expectation-falsification.test.ts is the "what
enumerates N" completeness gate: a new named expectation with no
fixture fails it. Counterexamples cover swallowed lifecycle command
prefixes, unsupported flags/selectors, pseudo refs, shell operators,
and invalid positional ordering.
- help-conformance-runner-output.mjs now returns a discriminated
RunnerOutcome ({kind:'success',commands}|{kind:'runner-error',
message,reason}) instead of a raw-string success inference. Only a
'success' outcome ever reaches validatePlanCommands/scoreExpectations
in runCase, so a runner-error result can no longer also carry
model-validation checks, and an all-runner-error aggregate now
reports passRate: null (rendered as "N/A") instead of "0/0 (0%)".
Fixes #1481
* refactor(bench): dedupe RunnerOutcome construction, drop leftover narrowing
Thermo-nuclear pass over 4b2df0a38's diff:
- help-conformance-bench.mjs's runOutcome() catch block was hand-building
the exact {kind:'runner-error', raw, message, reason} shape that
runner-output.mjs's private runnerError() helper already constructs for
its own two error paths. Export it as runnerErrorOutcome so the
discriminated union has exactly one constructor for its error variant,
reused by both error sources instead of duplicated.
- runCase's two return branches repeated the same
{runner, caseId, trial, outputPath} fields; pulled into a shared `base`
object.
- Reverted bench.test.ts's rateLimitedOutcome block: it had an explicit
`: RunnerOutcome` annotation and an if/throw narrowing guard, added only
to give fallow's dead-code checker a "real consumer" of the type before
the actual fix (adding the .d.mts to .fallowrc.json's ignorePatterns,
matching the existing sample-outputs.d.mts precedent) was found. That
workaround is now unnecessary scaffolding — replaced with the same
flat assert.deepEqual style the surrounding assertions already use.
|
||
|
Michał Pierzchała
|
735ab7672a |
refactor(daemon): one capture-input builder and one admit-then-bind step (#1876)
Behaviour-neutral. No descriptor changes platform execution, the cutover table is untouched, and no contract surface is added. - buildRuntimeCaptureInput moves to its own module so every request-bound capture consumer builds CaptureSnapshotInput one way. - The admit-then-bind sequence in the snapshot/diff resolver becomes one named step, ready for the selector units' second caller. - handlers/find.ts splits into focused target-capture and match-resolution concepts (600 -> 346 lines); behaviour unchanged. Co-authored-by: agent <agent@local> |
||
|
Michał Pierzchała
|
d07b837621 |
test: classify the runner XCTests — pure decisions to a macOS host lane, simulator semantics gated os(iOS) (#1781 A7) (#1861)
Every declared AgentDeviceRunnerUITests method now belongs to a lane, and the #if guard is the classification: AGENT_DEVICE_RUNNER_UNIT_TESTS alone means a pure runner decision (runs on the macOS host on every PR — ci.yml's existing compile job now executes the bundle it builds), '&& os(iOS)' means runner/XCTest semantics (simulator lanes only). check:xctest-selection evaluates the guards per platform, derives each lane's reach, and fails on a flagged identifier that is undeclared or uncompiled on that lane, on a declared test no lane reaches (found the two tvOS-only tests, dark since birth — widened to os(tvOS) || os(macOS)), and on testCommand reaching any lane. The host and nightly lanes assert executed == derived reach, so a missing -D flag or a guard that compiles a file out reads red, not as a smaller green. One duplicate test deleted (sparse-verdict assertions folded into its twin). |
||
|
Michał Pierzchała
|
f03c0309a1 |
fix: derive iOS transition snapshots from visible presentation (#1831)
* fix: project iOS transition semantics * fix: derive iOS transition semantics from visible state * fix: preserve iOS presentation context for scoped snapshots * fix: confirm broad iOS transition settlement * ci: run coordinate input regression on pull requests * test: mock migrated snapshot capture seam * fix: confirm transitions across snapshot backends * fix: arm transition confirmation after first capture * fix: settle against immutable action baseline |
||
|
Michał Pierzchała
|
6a8beb653e |
feat(mcp): compact server instructions in both eras + MCP-only help tool (#1839)
* feat(mcp): compact server instructions in both eras + MCP-only help tool (#1833) MCP-only clients got no workflow guidance: server/discover carried two sentences, legacy initialize carried nothing, and the CLI guides (agent-device --help, help <topic>) were unreachable over MCP. - MCP_SERVER_INSTRUCTIONS: one MCP-phrased workflow card (<2 KB, the Claude Code truncation limit) returned by server/discover and legacy initialize alike. - help tool, router-owned (not a command descriptor): no topic -> the CLI decision card; topic -> agent-device help <topic|command> text, prefixed with the one-line CLI->tool-property mapping; unknown topic -> isError listing the topics. listCommandTools() stays descriptor-only for the AI SDK; the router composes descriptors + help. - Move src/cli/parser/cli-help{,-overview}.ts to src/cli-schema/ so src/mcp (rank 3) can import the renderers without a layering back-edge into src/cli (rank 6). * fix(mcp): name terminal-only commands in help guides; colocate cli-help tests with their sources - The MCP guide preamble claimed every `agent-device <command>` line is a tool of that name; `help web` tells the reader to run `web setup` / `web doctor` and no `web` tool exists. The preamble now lists the exact CLI-only set (listCliCommandNames minus listMcpExposedCommandNames) — derived, not scanned out of prose where `device`/`web` are ordinary words. Regression: help web names `web` as terminal-only, and the listed set equals the registry difference. - cli-help-*.test.ts move from src/cli/parser/__tests__ to src/cli-schema/ to mirror the moved sources. * perf(mcp): tighten the guide card, tool description, and preamble Instructions card 1572 -> 1378 bytes (paid every session), tool description and preamble trimmed, HELP_TOOL built once as a const. Bundle delta vs main 3189 -> 2715 bytes; the remainder is the guide text itself, which the bundle carried in no MCP-phrased form before. |
||
|
Michał Pierzchała
|
142d156338 |
ci(ios): run the full XCTest suite nightly and check the PR test list (#1781 A7) (#1789)
* ci(ios): run the full XCTest suite nightly and check the PR test list (#1781 A7) * fix(ci): skip the runner server entry point in the nightly and validate both test flags * docs(ci): restate the nightly lane cost and timeout honestly * docs(ci): stop quoting XCTest counts that drift between commits * ci(ios): tighten the nightly timeout to the measured suite duration |
||
|
Michał Pierzchała
|
801734d433 |
feat(ai-sdk): add agent-device/ai-sdk tool set and document the MCP zero-code path (#1804)
* feat(ai-sdk): add agent-device/ai-sdk tool set and document the MCP zero-code path
Adds `createAgentDeviceTools()` under a new `agent-device/ai-sdk` subpath,
built from the same command registry the MCP server uses so both stay in
lockstep without a hand-maintained tool list. Introduces a `frameworkTier`
descriptor facet ('core' | 'extended') so the factory can default to a
curated perceive/act loop instead of handing a model dozens of tools.
`ai` is wired as an optional peer dependency, imported lazily inside the
factory rather than at module scope, so importing the subpath itself never
requires `ai` to be installed - only calling it does. The package's own
publishing gate (scripts/lib/shipped-imports.ts) is extended to recognize
peerDependencies as a valid resolution source, since this is the first
optional peer this package has shipped.
Also restructures the AI SDK doc around three tiers (zero-code via
@ai-sdk/mcp, the new typed tool set, hand-written tools) and fixes a stale
`needsApproval` reference in favor of the current `toolApproval` API.
* fix(layering): classify src/ai-sdk as a rank-4 zone
The layering guard requires every src/<folder>/ to be explicitly ranked or
unranked; the new src/ai-sdk/ subpath (added in the prior commit) was left
unclassified, failing CI's Layering Guard job. It sits at the same tier as
client/compat/daemon-server/metro/remote/sdk - a public integration surface
consuming mcp (3) and core (2), imported by nothing else in the tree.
* fix(ci): cover, exempt, and pack the new ai-sdk subpath
Fixes the remaining CI failures on the ai-sdk subpath commit:
- Coverage: src/ai-sdk/index.ts had no dedicated unit test (only manual/
integration verification), so changed-line coverage sat at 6.9% against
the 70% gate. Adds src/ai-sdk/__tests__/index.test.ts (core vs 'all' tool
filtering, session/platform pinning and schema hiding, error
normalization, toolApproval passthrough) with createCommandToolExecutor
and createAgentDeviceClient mocked the same way command-tools.test.ts
does, plus a dedicated missing-peer-dependency.test.ts that mocks `ai`
itself to throw, isolated to its own file so it doesn't affect the other
tests' use of the real, installed `ai` package. Changed-line coverage is
now 29/29 (100%).
- Fallow Code Quality: src/ai-sdk/index.ts and examples/sdk/ai-sdk-tools.ts
are entry points with no in-repo importer (reached only via package.json
exports / run directly), and the new subpath's exports are unused
internally by design - both need the same treatment src/sdk/*.ts and its
examples already have in .fallowrc.json.
- Integration Tests: test/integration/installed-package-metro.test.ts and
src/__tests__/package-exports.test.ts each hand-list every published
subpath and smoke-check it from a real packed install; added ./ai-sdk to
both so the new subpath is actually exercised, not just silently passing.
* fix(ai-sdk): hide MCP transport/config fields from the model too
createAgentDeviceTools() only removed session and mcpOutputFormat from tool
schemas. stateDir was still model-visible and reached the shared executor
as client configuration, letting a tool call redirect into a different
daemon state directory - defeating the "one pinned session" guarantee the
factory exists to provide. includeCost and responseLevel are MCP
tool-config knobs in the same category, irrelevant to this adapter.
Widens the hidden-field set to session/stateDir/mcpOutputFormat/
includeCost/responseLevel, and now strips them from the runtime input
inside execute() too (not just the schema), so the guarantee holds even if
a caller bypasses schema validation. The schema-properties filter and the
input filter now share one omitHidden() helper instead of two near-
duplicate implementations.
Addresses the P1 review comment on #1804.
|
||
|
Michał Pierzchała
|
04613ae8d3 |
ci: keep Bundle Size job green on transient GitHub comment failures (#1795)
* ci: keep Bundle Size job green on transient GitHub comment failures The size measurement and job summary had already succeeded on PR #1789 (run 32050847506) when the PR comment write got a 503 during a GitHub incident and failed the whole lane. --post-comment now retries 5xx / 429 / network errors (4 attempts, 1s/2s/4s backoff) on both the list and write calls. If it still fails, it prints a ::warning::, appends a note to $GITHUB_STEP_SUMMARY, and exits 0. Other 4xx (bad token, missing permissions) stay fatal. * refactor: split GitHub response classification to satisfy fallow complexity gate * fix: reconcile uncertain comment creates instead of re-POSTing; add regressions Retry now wraps the whole list -> write cycle rather than each request, so a create whose response was lost (network error / 5xx) is re-listed on the next attempt and turned into a PATCH of the marker comment instead of a duplicate POST. Splits the retry/classify helpers under the fallow complexity gate. Adds scripts/__tests__/size-report-post-comment.test.ts (unit-core): spawns the real script against a stubbed fetch and pins uncertain-create reconciliation, transient exhaustion (warn + exit 0), and fatal 4xx (nonzero, no retry). SIZE_REPORT_RETRY_BASE_MS lets the tests skip real backoff. |
||
|
Michał Pierzchała
|
c7565cb1f8 |
refactor(snapshot): clean snapshot ownership (#1754)
* refactor(snapshot): clean snapshot ownership * fix(snapshot): address ownership review feedback |
||
|
Michał Pierzchała
|
7f5dbd2e50 |
chore: drive unused production exports to zero (#1743)
* chore: drive unused production exports to zero `pnpm check:production-exports` has been failing on main with 21 findings. Each was investigated rather than blanket-suppressed; they split three ways. Genuinely dead, deleted: - `androidDeviceForSerial` (android/adb.ts) had zero references anywhere, tests included. - `streamAndroidLogcatWithAdb` (android/logcat.ts) had no production consumer and only a guard-clause test; its `captureAndroidLogcatWithAdb` sibling is the published SDK surface. Removed with its options type and test. Test-only aliases over live siblings, collapsed: - app-log-resource-store re-exposed four bound store methods; production used only `resolvePath`, tests used the other three. The sibling screen-recording-resource-store exports just the store, so this now matches: one export, all consumers call `appLogResourceStore.x`. - device-claims re-exported `canonicalLocalDeviceKey` for a single test, while production imports it from device-claim-paths directly. Dropped the re-export and pointed the test at the canonical module. Real consumers the analysis cannot see, exempted with the reason: - The nine remaining `src/cli/commands/*Command` handlers are reached only through `dedicatedCliCommandHandlerLoaders`, the dynamic import() table in router.ts. `deviceCommand` already carried an inline suppression for exactly this; replaced it with one config entry naming the table that enumerates all ten, matching the existing daemon route-handler entry. - `resolveVitestMaxWorkers` (vitest.config.ts), `DEVICE_CLAIM_IN_USE_SAMPLE` (bench sample producers) and the capture-kit `createAppLogLiveHandle` facade export joined the existing entries that already record their exact shape. - `**/*.fixtures.ts` is now an ignorePattern: all 15 build doubles for co-located tests, several import `vi`, and none is imported by production source. Pattern-matching them as test infrastructure also keeps this class of finding from recurring. Gate now reports zero. Unit suite, layering, fallow audit, MCP metadata, build, bundle-owner and package checks all pass. * chore: scope the fixture exemption to unused exports Review feedback on #1743: `ignorePatterns` removes a file from every Fallow mode and rule, but the false positive here is only production-unused-exports. Moved *.fixtures.ts to an ignoreExports entry so fixtures stay inside health, dead-code and cycle analysis. Kept `exports: ["*"]` rather than today's three symbols because the property is per-file — no fixture module has a production consumer — so a new fixture symbol should not reopen the finding. check:production-exports still reports zero, and a full `fallow --summary` returns identical totals (2 dead-code / 4 dupes / 123 health) with and without the change, so nothing was newly surfaced or newly hidden. * test(fallow): prove fixture policy scope |
||
|
Michał Pierzchała
|
f5d9789764 |
feat: enforce local device claims and reconcile stale owners (#1735)
* feat: enforce local device claims * fix: address device claim review feedback * fix: persist canonical daemon claim state directory |
||
|
Michał Pierzchała
|
b0d4b40467 |
chore: stop publishing skills to npm (#1730)
* chore: stop publishing skills to npm * fix: align simulator skill startup * docs: align agent setup with open-first workflow |
||
|
Michał Pierzchała
|
c2c81549d9 |
feat: add simulator verification skills (#1716)
* feat: add simulator verification skills * chore: simplify simulator skills * docs: refine simulator skill guidance * test: guard simulator skill workflows * style: format simulator skill contract test |
||
|
Michał Pierzchała
|
cdc754e6ed |
perf: speed up iOS agent recovery and streamline CLI guidance (#1700)
* Avoid interactive children in parent taps * docs: streamline no-skill CLI help * perf: recover faster from sparse iOS trees * fix: preserve selector context for blocked parent taps * fix: preserve coordinate text-entry focus * fix: preserve thin parent touch targets * fix: fail closed for unscoped iOS typing * test: isolate replay lock fixture * test: share node integration process |
||
|
Michał Pierzchała
|
18291ba8e2 |
perf: collapse app-driving startup turns (#1693)
* perf: collapse app-driving startup turns * fix: align foreground open guidance |
||
|
Michał Pierzchała
|
9c25bc66f4 |
docs(cli): advertise open --foreground and snapshot --actions in the workflow card (#1682)
* docs(cli): advertise open --foreground and snapshot --actions in the workflow card open --foreground (#1670/#1671) and snapshot -i --actions (#1665) shipped with no mention in the compact `help workflow` card, so a planning model never discovers either. Add one terse line each: the foreground fast-path in Bootstrap, and the merged-element custom-action guidance in Validation and evidence. Stays under the 9,000-byte compact-card budget (8493 -> 8908 bytes). Adds two help-conformance bench cases per the repo's changed-guidance rule: foreground-attach-single-sim (correct plan starts with `open --foreground` in an unambiguous single-sim scenario, fail-closed alternative forbidden) and merged-card-actions-not-directly-invokable (a merged Bluesky-style feed card's actions list is evidence, not a selector). Both use a real pinned sample rebuilt through the production snapshot renderer. * fix(scripts): accept flag order in the foreground-attach conformance matcher Flag order after `open` isn't semantically meaningful (`open --platform ios --foreground` is exactly as correct as `open --foreground --platform ios`), but startsWithForegroundOpen required --foreground to be the literal next token after `open`. Rescoring the completed repeat=3 bench report shows this docked codex:gpt-5.4-mini on all 3 trials even though its plan was config-order noise, not a real deviation -- the no-positional/no-device guarantee already comes from the forbidden checks. Loosened to require --foreground anywhere on the open line; foreground-attach-single-sim now scores 54/54 across both runners. * fix: close workflow help conformance gaps |
||
|
Michał Pierzchała
|
a158434a9c |
feat(cli): compact workflow help card + version header (#1663)
* feat(cli): compact workflow help card + version header Shrinks the per-task agent protocol tax of the help/skill surface. `agent-device help workflow` drops from 41025 to 8466 bytes (-79%) by moving depth into new `help scripting` (save-script, secret-safe fills, batch JSON, replay divergence/repair) and `help gestures` (multi-touch shapes/quirks) topics, and folding a few paragraphs into topics that already owned the subject (help debugging, help physical-device, help validate). Content is moved, not deleted. Every `help <topic>` first line is now `agent-device <version> — <topic>`, so the skill router reads the CLI version off the mandatory first help read instead of a separate `agent-device --version` call. SKILL.md is updated to do that and stays a thin router otherwise. The compact card also gains two terse behavioral rules: chain confident consecutive steps with `&&` (falling back to one command at a time when uncertain), and confirm the requested end state is actually visible on screen before declaring a task done. help-conformance-bench (22 cases x 2 runners) improves after the change: 29/44 -> 32/44 passing checks. * fix(cli): review follow-ups on the compact workflow card (#1663) Three fixes from PR review: - Extend the help-conformance plan validator to split a command line on unquoted && and validate each chained segment independently, so a plan that follows the workflow card's "chain confident consecutive steps with &&" guidance is accepted instead of rejected as one shell-projection violation. && inside a quoted selector value (e.g. label="A && B") is not a chain boundary and does not split. Adds unit tests for the splitter and a chains-confident-consecutive- settle-steps conformance case. batch stays out of this: it is deliberately stop-only. - Replace the literal @ref placeholder the compact card used in its own "snapshot -s @ref" example with a concrete ref (snapshot -s @e12 (the current concrete ref)), matching the same card's rule against placeholder targets. Reverts the test to demand the concrete shape. - Give help scripting and help gestures real conformance cases instead of waivers: a secret-safe recorded-fill + publish case, and an Android transform-then-verify case whose exact verification text only appears in the gestures topic. Removes both waivers. help-conformance-bench (25 cases x 2 runners, repeat=1) after these fixes: two full runs landed at 32/50 and 33/50. That is on par with the pre-change baseline (29/44) once the topic-untouched cases' run-to-run swings are accounted for (confirmed noise: one case with zero exposure to any change here flipped 10/10 -> 1/10 on a runner API error, and another swung across all three post-fix runs). The new scripting case now passes 8/8 for both runners; the new chaining case correctly reports the model's choice not to chain as a soft signal, not a validator failure. * fix(cli): update session.test.ts help pointer for moved script-authoring content * fix(cli): reject empty && chain operands in the plan validator (#1663) splitOnUnquotedAnd() previously trimmed and filtered out empty segments, so a plan with a leading (`&& press ...`), trailing (`press ... &&`), or doubled (`a && && b`) operator passed validPlanCommands even though a real shell rejects all three as a syntax error. The validator would bless a plan that fails at execution. Empty segments are now surfaced as an `empty-chain-operand` issue instead of being silently dropped. The quoted-&& non-split behavior (label="A && B") is unchanged, and a normal single command with no chain still parses identically to before. Adds regression tests for all three empty-operand shapes plus the quoted-&& case. |
||
|
Michał Pierzchała
|
3937036e5e |
feat: support --settle on scroll and back (#1638) (#1650)
* feat: support --settle on scroll and back (#1638) Scroll-then-observe and back-then-observe are legitimate agent pairs, but the post-action observation registry never grew past the touch commands, so `--settle` on either was rejected with INVALID_ARGS — burning a tool call each in AppControlBench's bsky-16. Both commands now carry the `settle` descriptor trait, and every surface derives from it rather than a hand list: CLI allowed flags, MCP/SDK input fields, the flag-sourced timeout envelope, and MCP ref-pinning. The CLI flag/metadata helpers moved out of the interaction family into post-action-observation-grammar.ts (back is a system command), and SETTLE_REF_ISSUING_TOOLS became a derivation — a hand list would have silently stopped pinning the new commands' refs. settleAfterInteraction and the new settleObservationCommand are two entry points over one engine: same loop, storage, hints, and diff bounds, with the target-less path supplying its own baseline and no proximity point. The daemon reaches that command through the runtime surface, never by importing `commands/` (R2) — the same seam the touch handlers use for press/fill — and generic-settle.ts is loaded through a lazy `await import` returning a closure, so the interaction runtime subgraph stays out of this dispatcher's static graph (a static edge folded ~18 files into the daemon-server type cycle; R10 caught it). Both of generic-settle's orderings are load-bearing and tested: the baseline is frozen before dispatch (and before the Android dialog preflight), and the observation runs after markDeferredInteractionOutcome so settle's first capture folds in the #1542 stabilization rather than racing it. The ADR 0014 "a settled diff publishes refs" rule moved to settle-ref-issuance.ts, shared by both routes. One divergence is deliberate: scroll/back resolve no element, so the diff baseline is the session's STORED pre-action tree — "settled tree vs the last tree you observed" — not press's freshly resolved pre-action capture. Both commands also switch to preserve-daemon on timeout, which changes the non-settle path too: with --settle their dominant hang mode is now a wedged accessibility bridge, and a timed-out capture must not reset the daemon and lose every session (#1105). The reviewed-set gate records it. Live-validated on an iOS 26.2 simulator (Settings): scroll --settle settled in 1786ms with a +6/-6 diff carrying fresh refs; back --settle in 771ms with +15/-6. Alternating cost runs, one call vs the pair it replaces: scroll 2.9-3.0s vs 5.3-5.6s, back 3.1-3.2s vs 4.7-5.1s. Those include the #1627 deep-capture extension. * fix: render settled-diff refs paste-ready in CLI output A settled diff activates a PARTIAL ref frame (ADR 0014), which admits only the pinned `@eN~s<gen>` form of the refs it issued. The unchanged-interactive tail already rendered that way, but the diff's own added lines rendered the bare `@eN` embedded in the snapshot line — so a CLI caller who copied the ref the diff just handed them got `plain_ref_requires_complete_frame` and had to append the generation by hand. Added lines now render pinned when the response carries `refsGeneration`, exactly like the tail. Removed lines render verbatim: they name elements that just left the screen, and `SettleDiffLine` never gives them a ref. This is not new to scroll/back — press/click/fill/longpress had the same gap since #1101. MCP was never affected: its ref-pin store rewrites plain refs on the way in, which is why the model never sees a suffix. Live: `scroll down --settle` now emits `+ @e14~s218078 [cell] "Game Center"`, and `press @e14~s218078` copied straight out of that line taps successfully. * test: record the pinned-diff-ref bytes in the output-economy baseline Rendering added diff-line refs pinned costs 8 bytes in the two settle CLI text samples (two `~s<gen>` suffixes). The output-economy baseline is the tripwire for exactly this, so the increase takes an explicit reviewed waiver rather than a silent baseline bump — the same one the settled TAIL's pins already carry, for the same ADR 0014 reason. Only `bytes` moves: lines, refs, hints, and shape are unchanged, which is the evidence that this is a suffix on existing refs and not a new payload. Caught by CI, not locally: `pnpm test:unit` runs unit-core and subprocess-stub only, while the Coverage lane runs every vitest project. * test: prove the generic settle degrades when its runtime cannot be built `createGenericSettleRuntime` catches and returns undefined so an observation that cannot even start does not fail an action that already succeeded. That was a claim in a docstring with nothing behind it — the one changed line the coverage gate reported uncovered (95/96). The test puts the session in the state the catch exists for: the router handed us a session that is no longer in the store, so building the settle runtime throws SESSION_NOT_FOUND. The response keeps its scroll result and simply carries no settle payload. Removing the try/catch fails it. * build: teach fallow that vi.mock reaches pinOwnProcessStartTime dynamically Not from this PR: #1642 added `pinOwnProcessStartTime` on main, and its three consumers reach it the only way a Vitest module mock can — `vi.mock(path, async (importOriginal) => (await import('...')).pinOwnProcessStartTime(...))`. Dependency analysis cannot follow that dynamic import to a consumer, so the export reads as dead the moment any PR pulls that file into its audit scope. This PR is the one that did. The entry records the consumers by path and the reason, matching the daemon route-handler entry directly above it, which exists for the same dynamic-`import()` limitation. * refactor: adopt the best of the parallel #1653 implementation Two sessions independently built #1638 (PR #1650 and PR #1653) and converged on the same architecture — trait in the registry, one engine with two entry points, runtime-command seam, lazy import, preserve-daemon, stored-baseline honesty. #1650 continues; this folds in what #1653 did better: - The agent-facing help core loop (cli-help.ts) now names scroll and back as settle-capable. Without this, the benchmarked closed-grammar help line kept instructing agents that --settle is only for press/click/fill/longpress — actively steering the AppControlBench models away from what #1638 shipped. - issueSettleRefs moves into session-snapshot.ts, beside the partial-frame primitive it wraps, deleting the single-function settle-ref-issuance module. - Their seam tests: back reader→writer settle plumbing, back CLI settle rendering, and a trait-less generic command (home) ignoring a stray settle flag rather than observing or rejecting. What #1650 had that #1653 lacked, for the record: the SETTLE_REF_ISSUING_TOOLS registry derivation (without it, MCP never pins a scroll/back settle diff's refs and the partial frame rejects every follow-up), BackCommandResult.settle in contracts, back's MCP output schema, paste-ready pinned diff refs, and the docs/changelog/baseline surfaces. * bench: help-conformance case for settled scroll-to-find planning The #1638 extension of the closed --settle grammar to scroll/back is the feature's entire payoff — collapsing scroll-then-observe into one call — and the closed command list is an enumerated N whose enumerator is this bench. The regex over the help text proves the sentence exists; this case checks whether a model plans differently because of it. One focused case, deliberately not coached: a pinned visible-first snapshot (rendered by formatSnapshotText, pinned by the sample-producers gate) whose wanted row is summarized off-screen with no ref anywhere in the output. The tempting pre-#1638 plan is `scroll` plus a separate `snapshot -i`; acceptance is the single settled call. Scoring was verified against eight plan shapes in both directions before recording. Model-backed record (claude-haiku-4-5, 3 trials, current help): 0/3 — but the decomposition is the finding. Settle eligibility GENERALIZED (3/3 trials put --settle on scroll unprompted; the mutation-suffix framing concern did not materialize) and the two-call habit is residual (1/3). All three trials failed on `scroll @e3 down --settle` — the pre-existing #1366 scroll-takes-no-target confusion, which the live CLI recovers with a dedicated hint but a single-shot bench cannot. The recorded gap is therefore a first-30 doc gap (nothing teaches that scroll takes no target), not a settle-eligibility gap; tuning the case until it passes would just delete the evidence. |
||
|
Michał Pierzchała
|
8ba5f9b8de |
fix: surface AMBIGUOUS_MATCH candidates and name find's supported actions (#1602)
* fix: surface AMBIGUOUS_MATCH candidates and name find's supported actions (#1597) AMBIGUOUS_MATCH errors now list the matching candidates (ref, role, label/identifier) rendered the same way as snapshot -i lines, capped at 5 with a "+N more" marker. buildAmbiguousMatchError (the single producer, src/daemon/handlers/find.ts) reuses formatSnapshotLine to build the list; formatAmbiguousMatchCandidateLines (src/utils/output.ts) renders it unconditionally on both text surfaces an agent actually reads (CLI printHumanError and MCP formatToolErrorText) — previously the candidates lived only in details, which neither surface printed. find's "Unsupported find action: X" (e.g. from `find <text> press`) now attaches a hint naming every action find actually supports and the two-step recovery shape: run find "<text>" to resolve the ref, then dispatch the gesture as its own command (press @eNN). The hint is a single exported constant (UNSUPPORTED_FIND_ACTION_HINT) shared by both throw sites — packages/selectors' raw-token parser and the CLI's typed reader (src/commands/interaction/selectors.ts) — so they can't drift. Matching semantics are unchanged; ambiguous rejection stays by-design. The help-conformance corpus's AMBIGUOUS_MATCH quiz is updated: its premise ("candidate refs were not shown") no longer holds, but with 3 identically-labeled candidates the lesson (don't guess a specific ref) still holds. * fix: guard the AMBIGUOUS_MATCH candidate renderer against device-domain shapes Review on #1602 (P2): formatAmbiguousMatchCandidateLines ran for every normalized error and stringified details.candidates unconditionally, but device-domain AMBIGUOUS_MATCH/APP_NOT_INSTALLED errors (findBootedAppleSimulatorWithApp, src/core/dispatch-resolve.ts) reuse that key for { id, name } device objects with no `matches` field — CLI and MCP would have printed "Candidates: [object Object]" for those. The renderer now requires numeric details.matches AND every candidate to be a string before rendering anything, restricting it to buildAmbiguousMatchError's element-match shape; unrecognized shapes render nothing, same as before this feature existed. Added regression tests against the exact device-error shape on both text surfaces. Also unexports AMBIGUOUS_MATCH_CANDIDATE_LIMIT (fallow flagged it as an unused production export) — it has no consumer outside find.ts. |
||
|
Michał Pierzchała
|
80feff42d6 |
build: verify the published tarball instead of grepping the bundle (#1578)
* build: verify the published tarball instead of grepping the bundle Replaces the bundle-dependency grep with one gate that packs the tarball npm would publish and proves it sound from a clean consumer install: publint and attw on the tarball, a two-way dependency-closure audit, an import of every `exports` subpath, and the CLI smoke run — all from outside the workspace, where no pnpm link can mask an unresolvable specifier. Also stops the build from emitting a publishable bundle in the first place: a missing workspace link now fails `pnpm build` instead of warning and exiting 0, which is how 0.20.4 shipped an unresolvable `@agent-device/ad-script` import. publint found 12 real defects in the current package — every `exports` entry listed `types` after `import`, so TypeScript resolved declarations by accident rather than by condition. The dependency audit found `pngjs` declared as a runtime dependency while tsdown inlines it, an install every user paid for and no shipped code reached; it moves to devDependencies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NamFJUgn9DGHrT2za11JbD * fix(ci): run the package gate without pnpm on the Node floor pnpm 11.17 requires Node >= 22.13, so `pnpm check:package` could not start on the 22.12 floor the Packaged CLI job exists to cover. The gate needs only `node` and `npm`, so the job invokes the script directly. Splits the dependency-closure audit into a collector and a message builder to clear Fallow's complexity threshold, and classifies both packaging linters in ignoreDependencies: they are subprocess CLIs with no importable API here, which dependency analysis cannot follow to an import. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NamFJUgn9DGHrT2za11JbD * fix(publishing): read every literal resolution form in the closure audit The dependency-closure audit derived shipped imports from the ESM module record alone, so it could not see a package resolved through `require` or a `createRequire` result: neither produces a module-record entry. A lazy `createRequire('@agent-device/…')` would therefore clear the audit, the all-export probe and the exercised CLI paths, reintroducing the 0.20.4 published-install failure class for another command. Measuring the built bundle turned up a second, larger hole in the same reader. The shipped files are minified, and the minifier rewrites every string literal to a no-substitution template literal, so the dynamic-import extraction — which accepted quoted strings only — matched 0 of the 99 dynamic imports the bundle contains. The lazy `import()` path that broke 0.20.4 was reported as covered while checking nothing. Specifiers now come from the module record plus an AST walk over every literal runtime-resolution form: `import()`, `require()`, `require.resolve()`, an immediately-invoked `createRequire(...)`, and calls through a `createRequire` result under any import or minified alias. Both spellings of a string literal count everywhere, and `.cjs` joins the scanned extensions. Computed specifiers stay explicitly out of scope, and are pinned as such. Rejecting them is not available: minifiers reuse short identifiers across scopes, and the packed bundle really does contain an unrelated `a(h[t],f,g,l,e,m)` that no name-based match can distinguish from a require call. Those are covered by the gate's runtime half instead, which resolves them for real. Bare-identifier calls need the one-string-argument shape for the same reason. The audit moves to scripts/lib/shipped-imports.ts so fixture packages can exercise it. The gate needs a real `npm pack` behind minutes of Swift and Android builds, so every check that runs it can only watch a healthy package pass — which is how a reader that matched nothing looked covered. The new fixtures assert the failure direction per resolution form: 16 of the 22 fail against the previous reader, and the 6 that pass are the quoted-spelling and pinned-limitation cases. A wiring assertion keeps the audit and both runtime probes attached to the gate, since fixtures alone would stay green if the call were deleted. Verified against the real built bundle: the closure resolves to exactly the two declared dependencies, so the stricter reader adds no false positives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NamFJUgn9DGHrT2za11JbD --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
2e74b789fd |
feat: verify device cloud connections (#1564)
* feat: verify device cloud connections * refactor: unify connect provider adapters * refactor: separate connect verification facts * fix: tighten connect provider verification * fix: use neutral cloud connection wording * perf: deduplicate local affected checks * refactor: simplify affected check runner * refactor: derive connect workflow from verification |
||
|
Michał Pierzchała
|
76453add71 |
refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) (#1494)
* refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) Extend the workspace with packages/* and move the kernel behind an enforced public API: packages/kernel with nine consumer-earned subpath exports (errors, device, snapshot, contracts, collections, rect, redaction, daemon-error, bounds — the last absorbed from utils as Rect vocabulary). Every kernel import repo-wide becomes the @agent-device/kernel/<sub> specifier; kernel tests move to src/__tests__/kernel/ and exercise the package surface. The root declares the package in devDependencies (workspace:*), tsdown bundles it (noExternal) so the published artifact and its runtime dependency manifest are unchanged. Gate rewiring in the same change, per the W0 brief: - R1 kernel-sink retires (physically subsumed); new R11 package-boundaries guards no-root-back-imports, relative tunnelling past exports maps, undeclared workspace deps, and non-exported subpaths, with runtime resolution pins via import.meta.resolve. - resolveImportEdges and mutation ownership follow workspace specifiers through exports maps, keeping R4 cycle checks, depgraph, and derived test ownership connected across the seam (kernel-errors still owns 495 tests). listSourceFiles includes packages/*/src. - kernel becomes an unranked zone; mutation registry, stryker mutate globs, and the mutation-affected workflow path filter move to packages/kernel/src/errors.ts. - check:affected gains packages/ ownership (manifests fail open); vitest and coverage include packages/*/src; fallow ignores packages/** (its resolver cannot follow workspace specifiers). - The affected-selector CI job installs dependencies: its closure now crosses workspace specifiers, and the R8 relative exception is unsafe for production src files (Node ESM does not realpath, so dual specifier/relative loads would instantiate modules twice). The R8 zero-dep set is pinned empty with that rationale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: address W0 review — mutation sandbox, exports-map resolution, tsc -b Review findings on #1494, all five: 1. contracts-schema-public.test.ts reads the kernel source at its packages/ path (fs access invisible to the codemod and typecheck). 2. Mutation lane: Stryker sandboxes the tree but pnpm's node_modules symlink resolves @agent-device/* back to the real repo, so mutants in the sandbox never load and vitest.related finds no tests. vitest.mutation.config.ts now aliases each EXPORTED specifier to its source (derived from exports maps, never a wildcard), keeping resolution inside the mutated tree. Validated: kernel-errors module runs end to end (dry run 3,984 tests, mutants killed, exit 0). 3. Layering/depgraph resolve workspace specifiers through the exports-derived map (workspaceSpecifierTargets) instead of reconstructing paths, so '.'-facade packages resolve; the positional fallback remains only for map-less fixtures (P0 pin). 4. Per-package project references implemented: packages/kernel is composite (emitDeclarationOnly -> dist-types, gitignored), the root references it, and typecheck becomes tsc -b — probed to catch type errors on both sides under TypeScript 7 native. 5. R11's relative-route exception now requires membership in an actual R8 zero-dep job closure (zeroDepClosureFiles walks entries), not mere scripts/ placement — closing the dual-instantiation bypass. Also from review discussion: daemon-error moves out of the kernel package to src/client/ — its consumers (cli, client facade) rehydrate wire DaemonErrors client-side; the daemon only produces them. Kernel drops to 8 exported subpaths before any of them ship. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * refactor: one exports-map reader for mutation alias and ownership Fallow flagged workspaceExportAliases (cognitive 15, CRAP 90). The manifest-reading logic already exists as workspaceSpecifierTargets in scripts/layering/package-boundaries.ts, so both the Stryker sandbox alias table and the mutation ownership walker now consume it instead of carrying near-clones. Behavior unchanged; mutation suite 45/45 and changed-code fallow green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: composite kernel without a root references edge FreeRange runs plain `tsc -p tsconfig.json`, and a root `references` entry makes non-build-mode TypeScript demand the referenced project's built declarations (TS6305) — a standing "build first" tax on every plain -p consumer (fr, editors). Keep the per-package composite project and build it in typecheck (`tsc -b packages/kernel` before the root and examples/sdk passes), but drop the root references edge: root consumption resolves through exports to source, identical to runtime and to the bundler. Probed: plain -p green with no prebuilt output; kernel-side type errors still caught by its own build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: R11 uses the layering parser; mutation config is a fallow entry Review blockers on #1494: - R11's private single-quote regex could miss a double-quoted or re-export route into packages/*/src. specifierSites now delegates to the layering model's parseImports (both quote styles, side-effect imports, re-exports, dynamic imports), with direct regressions for each formerly-invisible form. - vitest.mutation.config.ts becomes a declared fallow entry instead of a tolerated unused-file finding: the full-repo audit now reports it reachable (unused files 2 -> 1; the remainder predates this PR). FreeRange clean-checkout evidence: with packages/kernel/dist-types and every *.tsbuildinfo deleted, `pnpm check:freerange` reports 0 findings on this head — the TS6305 topology died with the root references edge in the previous commit; check:freerange has no build precondition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
devin-ai-integration[bot]
|
7402a40bac |
test: enumerate error-code recovery quizzes in a unit-lane gate (#1445)
* test: enumerate error-code recovery quizzes in a unit-lane gate Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: mark recovery quizzes structurally and derive retriability from the enumeration Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
2d1d70613f |
feat(bench): renderer-pinned samples, topic-coverage gate, error-recovery quizzes; trim skillgym to agentic checks (#1411)
* feat(bench): renderer-pinned samples, topic-coverage gate, error quizzes; trim skillgym to agentic checks The help conformance bench's quoted CLI output is now sourced from scripts/help-conformance-sample-outputs.mjs, and every sample is rebuilt through the real production renderers (settle output formatters, printHumanError, formatSnapshotText, refMutationAdmissionResponse) by scripts/__tests__/help-conformance-sample-outputs.test.ts — a rendering or message change fails deterministically instead of leaving the bench grading against output the CLI no longer prints. This retires the fabricated recoverable-failure envelope (production never throws a textual settle timeout; that case is replaced by a real DEVICE_IN_USE recovery quiz). Bench cases move to scripts/help-conformance-cases.mjs and are enumerated against the help-topic registry: helpTopicIds() is exported from cli-help, and scripts/__tests__/help-conformance-topic-coverage.test.ts fails when a help topic has neither a bench case nor an explicit waiver. New case families: error-envelope recovery quizzes (device-in-use, stale pinned ref, ambiguous find match, app-not-installed) pinned to real error text, topic coverage for tv/web/react-native/debugging/workflow, and a metamorphic twin of the settled-diff quiz. The skillgym smoke suite shrinks from 119 cases to the 5 that measure what only an agentic runner can show: skill routing plus output interpretation with a proven local CLI help probe (local-cli-help-policy). Its embedded samples now import the same pinned constants, replacing hand-transcribed output that had already drifted from the renderer. Knowledge checks belong to the bench; live fixture behavior belongs to the iOS simulator e2e suite. * review: drive error samples through the real producers; enforce local-help on the routing smoke The DEVICE_IN_USE, AMBIGUOUS_MATCH, and APP_NOT_INSTALLED parity tests no longer hand-author the producer message before rendering: each drives the actual producer — buildDeviceInUseBySessionError (extracted in session-open.ts and called by the handler), buildAmbiguousMatchError (now exported from find.ts), and buildAppNotInstalledError (extracted in app-resolution.ts and thrown by the resolver). Because each factory is exported from its producer file and called by the production path, dropping the production call would make it test-only and fail check:production-exports — the wiring is gate-enforced, not conventional. open-and-snapshot now sets requireLocalCliHelp and allowOnlyLocalCliHelpCommands, so the 'skill plus local help' claim is observed rather than assumed; without them the case can pass on model prior alone. |
||
|
Michał Pierzchała
|
ab913c9720 |
feat: strengthen agent help benchmarks (#1404)
* feat: strengthen agent help benchmarks * fix: harden help benchmark review findings * fix: close help benchmark validation bypasses * fix: make selector scoring quote-insensitive |
||
|
Michał Pierzchała
|
983625fc5d |
feat: fix codex runner, add --override-doc grading, port skillgym quiz cases to help-conformance bench (#1176)
* feat: fix codex runner, add override-doc grading, port skillgym quiz cases The 2026-07-09 evaluation of scripts/help-conformance-bench.mjs found it structurally right but broken for the codex runner (two bugs), thin on coverage (4 cases), sequential, and unable to grade a draft help rewrite without a rebuild. - Fix runCodex: (1) codex exec reads stdin until EOF when not attached to a TTY, and execFile never closes the child's stdin, so every codex call hung until RUN_TIMEOUT_MS with empty output — close stdin right after spawn. (2) `-o outFile` writes the same final JSON that codex also prints to stdout, so concatenating both produced two back-to-back JSON objects that broke every JSON.parse candidate and silently zeroed extractCommands() — prefer the clean -o payload, fall back to stdout only when it's empty. - Add `--override-doc <topicId>=<path>` (repeatable): loads a topic's text from a file instead of shelling out to `node bin/agent-device.mjs help <topic>`, so a draft help rewrite can be A/B graded with zero rebuild. - Port three cases from test/skillgym/suites/agent-device-smoke-suite.ts (settle-diff-is-observation, sample-output-settled-diff-next-target, sample-output-not-settled-needs-observe) as self-contained "next-command quiz" cases, generalizing the scorer to support regex matchers/forbidden patterns alongside the existing named expectations. Fixture output text matches the CURRENT settle rendering in src/commands/interaction/output.ts, including the "unchanged interactive (N):" tail added by #1167/#1172. - Parallelize the runner x case matrix with a concurrency cap (HELP_BENCH_CONCURRENCY, default 4); results still print in the original matrix order. - Extend test/skillgym/README.md's existing pointer to this bench with the new flags. Validated with real LLM calls (both runners, all 7 cases, 14 calls, ~$0.25 total): 13/14 pass; the one fail (claude-haiku-4-5 on dogfood-mode) is a genuine model miss (returned an empty command plan asking for the app name instead of committing to a generic plan), not a bench bug. `--override-doc` demonstrated live: stripping the dogfood doc's evidence-command examples regresses codex:gpt-5.4-mini from 3/3 to 2/3 on the same case, showing the flag both loads and changes grading. * fix: apply live-doc post-processing to --override-doc, fail fast on bad overrides Review findings on the initial version (all reproduced): - HIGH: an override for the --help:first30 doc id skipped the live path's firstLines(text, 30) cap, so a 49-line draft leaked lines 31-49 into the prompt — grading content a live run never shows, on the doc id every case uses. loadDoc now splits source (live shell-out vs override file) from post-processing, and the post-processing applies to both, so an override differs ONLY in where the text comes from. - MEDIUM: an --override-doc topic id no selected case uses was silently ignored (exit 0, real doc graded). Now fails fast listing the valid doc ids for the selection. - LOW: a missing override file threw a raw ENOENT stack trace; expected failures now print one clean Error line. Added --help usage text that documents last-wins semantics for repeated same-topic overrides and the post-processing parity. Guard tests (scripts/__tests__/help-conformance-bench.test.ts, wired into the unit-core vitest project by explicit path): a 49-line fixture whose prompt must keep line 30 and drop line 31, unknown-topic fail-fast with valid ids listed, clean no-stack error for a missing file, and last-wins for repeated overrides. All spawn the script in --dry-run with every required doc overridden, so they need no LLM calls and no built CLI. Live re-validation: a 33-line override of --help:first30 whose lines 31-33 instruct the model to emit a sentinel command; neither claude-haiku-4-5 nor codex:gpt-5.4-mini emitted it (both scored 4/4, matching the live-doc baseline), proving the cap applies end-to-end. |