mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
docs/improvement-audit-tracker
1606 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michał Pierzchała
|
828787f62a |
docs: add verified improvement audit, tier work plan, and quality backlog
Records the 2026-08 adversarially-verified architecture audit (v2), the worker-wave execution plan it produced, and the repo-quality backlog flagged alongside it. All 12 execution PRs through wave B have merged. |
||
|
Michał Pierzchała
|
67b813c55b |
fix(web): launch npm and the managed backend through node, not .cmd shims (#2033)
On Windows every `--platform web` command failed with `spawn EINVAL`: the managed backend resolved to `node_modules/.bin/agent-browser.cmd` and was spawned with `shell: false`, which Node refuses for `.bat`/`.cmd` since the CVE-2024-27980 fix. `web setup` failed earlier still — a bare `npm` is not spawnable on Windows, where npm ships as `npm.cmd`. `runManagedAgentBrowser` is now the only path that executes the backend. Entry resolution, the Node runtime, the managed environment, and the spawn all live behind it, so setup, doctor, and the provider cannot reintroduce the shim. The entry comes from the installed package's declared `bin` rather than a hard-coded path, which is the part of this worth being precise about. npm is untouched on macOS and Linux, which were never broken: setup still spawns `npm` from PATH. Only Windows resolves npm's own `npm-cli.js` — from an `npm_execpath` that really is npm's launcher, else the copy bundled beside `node` — and fails with the existing actionable TOOL_MISSING when neither is there. Setup also pins `--no-global` so an ambient `npm_config_global` cannot redirect the install out of the managed prefix. The published status shape is unchanged: `binaryPath` still names npm's console shim, now informational rather than the spawned command, and `entryScript` plus `packageDir` are additive. Closes #2022 Claude-Session: https://claude.ai/code/session_01LMS3BidXb3F4HSr26vvQmG Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
8fa91ac41c |
fix(daemon): close no longer throws tenant-isolation error on a never-allocated lease (#2029)
* fix(daemon): close no longer throws tenant-isolation error on a never-allocated lease `close` on a tenant-isolated connection (e.g. BrowserStack) whose lease was never allocated (`connect` succeeded but `open` never reached `lease_allocate`) threw a generic "tenant isolation requires lease id." error from the lease-admission gate before `close`'s own handler ever got a chance to run — masking the much clearer SESSION_NOT_FOUND / lease-less teardown path it already supports. Scoped narrowly to plain `close` (no app-target positional) so `close <app>`, which resolves its device straight from flags when there's no session, still goes through full lease/tenant admission. Fixes #2016 * fix(daemon): require session===undefined for the close lease bypass, not just a missing lease field Review on #2029 (P1): the bypass fired on any lease-less session, including a *stored* SessionState. Tenant-scoped session names are keyed by tenant, not by run, so a lease-less stored session could belong to a different run in the same tenant — a caller without a matching lease could tear it down. Narrow the bypass to session === undefined (no daemon session record at all), which is the actual deferred-connect case from #2016: `open` never ran, so the daemon never created a session to protect. Adds router-level regression tests through the real session-scoping/locked-admission/close-handler pipeline (request-execution-scope.test.ts): deferred connect with no session closes as SESSION_NOT_FOUND without ever calling the lease provider, while an app-target close and an existing lease-less stored session both still require a real lease. * test: bump workflow help-card byte budget after merging main Merging main (#2020) grew the workflow help card's "Bootstrap" line past the existing 9000-byte budget by a few bytes — unrelated to this PR's fix, just picked up by merging latest main. Bumping the budget rather than trimming #2020's recently-rewritten help copy. * fix(daemon): move close's admission bypass into the command-descriptor registry Review on #2029, P1: request-admission.ts was reclassifying req.command and req.positionals inline instead of consuming a registry predicate (ADR-0003). Added a request-sensitive DaemonCommandDescriptor trait, sessionlessPlainCloseAdmissionExempt, declared on close's descriptor via a named predicate (isPlainCloseRequest); request-admission.ts now asks the registry (isSessionlessPlainCloseAdmissionExempt) instead of matching req.command === 'close' and req.positionals.length itself. P2: reverted the workflow help-card byte-budget bump from #2030bd (9000 -> 9100) back to 9000, and instead trimmed the "Bootstrap" help line by the minimum amount ("or one provider" -> "or provider", "providers never fall back" -> "no provider fallback") to fit the existing budget with a small margin (8994 bytes). That budget regression came from #2020 on main, unrelated to this PR's close/lease-admission fix. |
||
|
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
|
214d3b2799 |
perf(daemon): revalidate the source code-signature cache by stat instead of rereading the graph (#2004)
* perf(daemon): revalidate the source daemon code signature by stat
Every CLI invocation from a source checkout fingerprinted the daemon's
import graph to decide whether the running daemon still matches this code
(isReusableDaemonInfo). Rediscovering the graph's edges meant reading all
~800 modules: 3,915 statSync + 803 readFileSync (4.6MB) per invocation,
~30ms of a ~245ms command.
The walk is now cached under os.tmpdir() and revalidated by statSync alone.
That is sound against the walk it replaces rather than merely close to it:
the signature already treats a file's size:mtime as the stand-in for its
contents, so if every previously visited file still carries its recorded
pair, no file's contents changed, therefore no import specifier changed,
therefore the graph and its signature are unchanged. Any mismatched,
vanished, or non-file entry, and any unreadable or malformed document, falls
back to the full walk and republishes. code-signature.ts is itself inside the
graph it walks, so changing the walk invalidates every stored document.
The dist entry keeps the direct walk: it is a ~120-chunk bundle at ~5ms, and
a bundled install has no source graph to amortize.
Warm source invocation: 803 statSync, 1 read, ~1.5ms for the signature
(was ~30ms); the command drops from ~245ms to ~205ms idle, and from ~342ms
to ~251ms under a loaded host.
resolveDaemonLaunchSpec (~8 existence probes) and readVersion are memoized
per process alongside it; both are immutable for the life of a process. The
signature deliberately is not, so a long-lived client still notices a daemon
rebuilt underneath it.
* refactor(daemon): read the divergence resume through its contract type
The client's keep-alive check and the daemon's repair-liveness stamp each
reconstructed `details.divergence.resume` inline: one through ad-hoc
Record<string, unknown> casts, the other through a private structural guard
next to the stamp. ReplayDivergenceResume already owns that shape, so the
narrowing moves next to the type in @agent-device/contracts and both callers
read through it.
The client is now as strict as the daemon, which is what it always meant:
repairSessionHeld is only ever written to a record this reader accepted
(markSessionHeldIfArmed), so no payload that carries the R7 signal can fail
it.
* fix(daemon): refuse a code-signature cache document that is not this graph's
Addresses the findings in REVIEW.md against this branch.
F1 (high): a `{"version":1,"files":[]}` document validated forever — every
stamp in an empty list matches vacuously — so the client answered
`graph:0:da39a3ee...`, disagreed with the daemon's own walk on every
invocation, and killed a healthy daemon each time, permanently. A stored
stamp list is now refused unless it carries the entry's own label, and
publishing is gated on the same predicate, so a document the reader would
refuse is never written. The label comes from `buildDaemonCodeFileLabel`, the
function the walk stamps with, so the two derivations cannot drift.
F2 (medium): `os.tmpdir()` is the shared `/tmp` on Linux. The cache directory
is now per-user (`agent-device-code-signature-<uid>`, 0700, documents 0600),
so no uid silently turns the cache off for every other one; and a document is
read through one descriptor and refused unless `fstat` says this user wrote
it, so a planted document cannot choose the signature the client compares a
running daemon against.
F4 (low): `readReplayDivergenceResume` now checks the two optional fields it
types — `repairSessionHeld` (present means exactly `true`) and
`alternateFrom` (an integer) — so an accepted payload cannot type as the R7
liveness signal while carrying something else.
F5 (low): a launch-spec test stubs `--experimental-strip-types` to route the
source arm of `resolveLocalDaemonCodeSignature`, the branch the cache exists
for and the one Vitest never reached in a built checkout.
Also: the cache key hashes realpaths, as `buildSourceCheckoutStateDirName`
already does, so symlinked paths to one checkout share one document; the
module comment now states the real bound (as strong as the walk for edits to
files already in the graph, weaker for a module that joins it while every
recorded stamp still matches) instead of claiming exact equivalence; and the
cache fixture redirects `os.tmpdir()` into its own root so it no longer
clobbers documents in the run's shared TMPDIR.
Every new test was observed red against the code it guards.
* fix(daemon): keep the code-signature cache out of the startup closure
PR #2004 CI: the coverage lane's eager-closure gate failed on two rows.
`packages/contracts/src/facades/divergence.ts` (3 -> 4): the new
`readReplayDivergenceResume` reached for `isRecord`, and `./json.ts` was not
otherwise in that facade's closure. It now narrows structurally and locally,
the way this module's other wire readers (`divergenceStepLine`,
`divergenceScreenLine`, `divergenceOverflowLine`) already do; the accepted
payloads are unchanged, and the rejection table proves it.
`src/cli.ts` (362 -> 365): three modules, only one of which had a lazy seam.
`src/daemon/code-signature-cache.ts` is reached only by a source checkout, so
an installed client statically evaluated it -- and `src/utils/atomic-file.ts`
behind it -- on every invocation without ever being able to use it. It now
loads through a function-scoped `await import` in the source arm of
`resolveLocalDaemonCodeSignature`, which is therefore async.
Propagating that await collapsed the duplicated reuse ladder: the same three
checks were written twice, once as `isReusableDaemonInfo` and once as
`resolveDaemonTakeoverReason`, whose `'not reusable'` fallback was already
dead. One function now answers both questions -- the reason, or `undefined`
when the daemon is reusable -- so the verdict and the notice explaining it
cannot disagree, and the signature is still resolved only after the version
check passes.
The remaining two are deliberate and stay eager, so `src/cli.ts` is pinned at
364 with the reason recorded next to the row: `src/utils/ttl-memo.ts` (the
per-process version/project-root memo this PR added to cut re-reads of
package.json) and `src/daemon/client/daemon-launch-spec.ts` (the launch-entry
probe split out of daemon-client-lifecycle.ts). Both sit on the path every
local command already takes; deferring either would move the same load, not
avoid it.
* fix(daemon): make the code-signature cache sound across resolution changes
Extensionless specifiers resolve .ts before .js, so adding dep.ts to a graph
whose './dep' resolved to dep.js changed a fresh walk while leaving every
cached stamp untouched — the stale signature was silently reused. The walk now
records every candidate probed-and-missed ahead of a winner (and every
candidate of an unresolved specifier) as absent paths; the cache revalidates
that each is still missing and re-walks when any appears. Red-first:
cached-vs-fresh dep.js -> newly added dep.ts regression, plus lower-precedence
appearance staying warm and a nothing-resolving specifier gaining a target.
|
||
|
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
|
17cfd8ca8a |
feat: add deterministic device selection resolver (#2020)
* feat: add deterministic device selection resolver * test: adapt open selection harnesses * chore: keep context glossary within budget * fix: separate device identity from selection filters * refactor: make the selection resolver the sole owner of selection provenance Simplifies the deterministic device-selection resolver (net -114 lines vs the previous head) while fixing the outstanding app-aware provenance finding: - Move the booted-simulator app-affinity narrowing into the resolver behind an appleSimulatorAppTarget param, with its own typed reason 'single-app-installed-local' (candidateCount 1). This removes the selectedDevice escape hatch that reported 'preferred-local' with candidateCount 2 for the app-narrowed pick, and gives the app-match errors the same platform-aware retry selectors as every other selection failure. - Delete dead code: the allowBootableLocal param (no caller ever passed it, so the eligibleDevices filter was unreachable), the hasExplicitProviderIdentity alias, the duplicated deviceCandidateDetails in dispatch-resolve, and the double candidate computation. - Shrink the public selection contract to what #1777 specifies: drop `booted` (it contradicted its own doc comment once markSelectionBootedAfterPreparation flipped it; bootOccurred plus the reason codes carry the same information) and drop `retrySelectors` from success metadata (the daemon only ever emits retry selectors inside error details). DeviceSelectionRetrySelector leaves the contracts facade. - Replace the typeof-import lazy seam and resolver threading through four context objects with one lazy forwarding wrapper; the dispatch eager closure stays at 83. - Collapse the Apple path to resolve -> optional simulator fallback; the provider branch goes through the generic resolver call directly. - Consolidate the five copy-pasted resolveTargetDeviceSelection test mocks into one shared stub (selectionFromResolveTargetDevice). - Add the requested regression: two booted simulators with the app on one now assert typed selection metadata through resolveTargetDeviceSelection, plus a resolver-level app-affinity provenance test. Validation: typecheck, oxlint, oxfmt, layering (184-check guard OK), DI seams, fallow changed-files, eager-closure 235/235, daemon suite 323 files / 2287 tests, core/commands/mcp/client suites 255 files / 2133 tests. |
||
|
Michał Pierzchała
|
f7f22d9e82 |
refactor(android): admit text onto one resolved channel (#2030)
One module-private admitAndroidTextChannel owns the whole text-entry decision after the provider check: helper IME (from the activation cache or the observed active input method, package included) or the adb-shell channel, admitted only after the ASCII and input-ownership asserts. Callers make one call and one dispatch; the read-before-refuse ordering and both shell asserts are unreachable from outside the resolver. fillAndroid resolves the channel inside its attempts loop after focus, so the helper fill's first attempt reuses that focus instead of tapping again; only the retry re-focuses. This removes the double tap the observed-IME fill route used to make. Tests pin one tap on first-attempt helper fills and two taps when verification fails once. |
||
|
Michał Pierzchała
|
b5adf966b4 |
perf(android): one dumpsys read per window question; batch text via the helper IME (#2003)
* perf(android): answer every window question from one dumpsys read Blocking-dialog readiness ran `dumpsys window windows` AND `dumpsys window` on every check, because "this dump shows no ANR" and "this dump shows nothing" were the same answer. A tap paid that twice — before and after dispatch — and the ANR recovery poll paid it plus a separate foreground probe on every 500 ms tick, up to ~78 adb spawns. - `window-state.ts` owns the dump sequence for both questions the daemon asks of a window dump. `readAndroidBlockingDialogFocus` now reports whether the focused window was observed at all, so the second variant fires only on the miss it exists for, not on every clear screen. Marker order is unchanged. - An `AndroidWindowDumpReader` binds a caller to one observation instant. The ANR poll passes one to both probes, so a tick costs one spawn and cannot report a foreground package read after a dialog check that saw another screen. - `android-dialog-readiness-observation.ts` lets one command's post-dispatch check answer the next command's pre-dispatch check, keyed on the ADR 0014 session runtime revision (advances at every device side-effect seam) and a 1 s ceiling for dialogs that appear with no adb traffic. Only `after-command` writes and only `before-command` reads, so ANR-appeared-after-the-command detection is never skipped. A run of three taps on a clear screen: 12 window dumps before, 4 after. * perf(android): batch text entry whenever the device is on the helper IME The IME helper's broadcast channel writes a whole string in one `am broadcast`, but it was reachable only through this daemon process's activation cache. A device already carrying the helper as its active input method — left by a crashed run, switched by another daemon on the same emulator, or opened with `--no-test-ime` — silently fell back to ceil(n/8) `input text` spawns. Type and fill now read the active input method from the `dumpsys input_method` probe the shell path already runs, and route to the batch channel when that input method IS the helper. Zero extra spawns: the read that decides the channel is the read that already decided IME ownership. The broadcast targets the observed package, so the promoted route does not depend on a packaged artifact being on disk. `ANDROID_INPUT_TEXT_CHUNK_SIZE` stays at 8 and now says why in the code: it is the #531 truncation fix, not a tuning knob. `text-entry.ts` extracts the shell mechanics (chunking, encoding, typed unsupported/ime_capture classification, ownership) that `input-actions.ts` had outgrown; `assertAndroidShellInputIsAppOwned` splits into a probe and a pure assertion so one read can answer both questions. * perf(android): stop asking a dumpsys variant that never answers Live tracing on Android 16 found the real cost. `dumpsys window windows` does not print `mCurrentFocus` there at all — the focus section moved into the display dump plain `dumpsys window` prints — so the first command in the sequence is a ~53 KB transfer that structurally cannot answer, paid on every readiness check, foreground read, and ANR poll tick. `window-state.ts` now records, per serial, whether each variant has EVER printed a focus section, and demotes the ones that never have to the back of the sequence. "Ever answered" is the load-bearing part: a variant that normally answers still prints nothing during a window transition, and one such sample must not demote it. A demoted variant is still asked when nothing ahead of it answers, so a wrong record costs ordering, never an answer. The marker list is now shared with `app-parsers.ts` instead of restated, and the structural probe uses its own non-global regex rather than borrowing the foreground parser's `lastIndex`. Live, Pixel_7_review / Android 16, per tap: 6 window dumps before, 2 after. * docs(worker): record the B1 result and drop the brief from the tree * fix(android): close the REVIEW.md correctness findings on window readiness REVIEW.md's verdict was do-not-merge on four confirmed defects plus an unrebased tree. F8 is the rebase itself (previous commits); this closes the four correctness findings, each pinned by a test that was observed red first. F1 — `focusObserved` answered a different question than the one it short-circuited. Any of the four focus markers set it, including `mFocusedApp=AppWindowToken{`, which names the focused app token and structurally cannot carry an ANR window title. A device whose `dumpsys window windows` prints the app token but no `mCurrentFocus` therefore reported a live ANR as a clear screen and the command's tap landed on the dialog's buttons. Only `ANDROID_FOCUSED_WINDOW_MARKER` sets it now; the marker order and what the other markers parse are unchanged (#592). F2 — the demotion memo could invert an incident-derived priority list. One transition-instant read (WMS prints no focus during a window transition, on the lock screen, with the screen off) demoted both window variants, and the 4-command foreground sequence then let `mResumedActivity` answer ahead of `mCurrentFocus` — reporting a press that escaped into Settings as a success. The sequence is tiered now: every variant within a tier answers from the same source, so demotion reorders inside a tier and never across one, which is what "a wrong record costs ordering, never an answer" always claimed. The memo is also per question, because a dump that names the app token answers "which app is foreground" and can never answer "is a dialog blocking us". F3 — the 1 s readiness observation reuse is removed. Its cost was not "detection delayed by one command": on `main` the pre-dispatch check recovered a session-owned ANR and returned `{ status: 'recovered', warning }`, so the command proceeded and succeeded. With the reuse the same situation dispatched the tap into the ANR dialog and then failed the command hard. The post-check cannot restore that shape — it cannot tell an ANR that predates the dispatch from one the dispatch provoked, and guessing "predates" would report a command that froze the app as a success. Preserving `main`'s outcome means probing before every dispatch. Two dumps per tap instead of one, still 2x better than `main`. F4 — "the dump told us nothing" is no longer "the device was observed clear". `getAndroidBlockingDialogObservation` answers `dialog` / `clear` / `unknown` and `ensureAndroidBlockingSystemDialogReady` branches on all three. The command still proceeds on `unknown` (a failed probe has never been a refusal, on `main` either) but emits `android_blocking_dialog_unobserved`, and nothing about that dump is carried forward as evidence. RESULT.md records the revisions and what F5-F7/F9 leave open. * test(android): drop stray runner-client mock left by rebase conflict Main never touched this file since the branch base; the mock referenced an undefined hoisted variable and broke module load. * test(android): converge touch-direct suite onto the platform-runtime shape The ADR-0019 waves replaced the dispatchCommand-mocking click tests this branch carried; keep main's surviving coverage and adapt only the window- state readiness mock to the branch's observation API. * test(android): read ANR recovery through the Android capture envelope PR #2003's Coverage lane failed two cases in `android-dialog-readiness.test.ts` with "Android app ANR blocked tap: ... Automatic recovery failed." CI runs the branch merged into `main`, and #1981 landed on `main` after this branch's last rebase: Android publication now derives covered state from acquisition evidence carried in an opaque capture envelope, so `readAndroidSnapshotNodes` goes through `androidSnapshotPublicationInput`. This branch's new readiness test mocked `snapshotAndroid` with a bare `{ nodes }`, publication threw on the missing evidence, and the ANR recovery's own catch reported that as "recovery failed" - a fixture defect that reads exactly like broken recovery. Rebased onto `origin/main` and built the fake capture with the production constructor (`makeAndroidSnapshotCapture`), so the fixture cannot drift from what production publishes. No recovery guarantee is relaxed; two are pinned harder: - the recovered case now asserts the tap that dismissed the dialog, not the status alone, so a status reported without the device work behind it fails; - a new case pins #1832 on the readiness path - a "Close app" covered by a foreground surface is never tapped, and recovery reports failure so the command fails closed. Verified red-first by mutation: dropping the occlusion filter in `findCloseAppButton` makes the new case fail (it taps the covered button and reports recovery). Gates: `pnpm vitest run src/daemon src/platforms/android` (383 files / 2780 tests), full `unit-core` (1035 files / 7888 tests), `pnpm format`, `tsc --noEmit`, `pnpm check:affected --run`. * test(daemon): stub the dialog probe where this branch moved it PR #2003 CI, coverage shard: 'request router joins orientation admission to execution and ref invalidation' failed with `{ ok: false, error }` where it expects `{ ok: true, data }`. `orientation` carries `androidBlockingDialogGuard: true`, so the router runs the real `ensureNoAndroidBlockingDialogReady` check for this file's Android device. This branch moved `getAndroidBlockingDialogObservation` out of `app-lifecycle.ts` and into `window-state.ts`, which answers every window question from one dumpsys read. The sibling suites moved their stubs with it; this one kept aiming at the old module, where the spread only added a key nothing imports. The stub was a silent no-op and the real `adb` spawn ran. A host with `adb` on PATH still passed: no such device, empty dump, `unknown` status, warning path, `ok: true`. A runner without `adb` fails the spawn instead, and the guard's catch turns that into an error response — the shard's failure. Reproduced locally by dropping `adb` from PATH, which gives the CI assertion verbatim; green there after the fix, and the case drops from 939ms to 9ms now that it no longer spawns `adb` twice. Production is unchanged: the guard is correct to gate orientation, which is state-changing and would otherwise rotate behind a blocking dialog (#1832, #592). Only the test's seam pin was stale. * fix(android): read the active IME before refusing non-ASCII text PR #2003 review, product P1. `typeAndroid` and `fillAndroid` both ran `assertAndroidShellTextSupported` before probing which input method the device was actually on. That refusal is a property of the shell writer — `input text` is ASCII-only — but it was applied to every caller that had nothing in this process's activation cache. A device already carrying the helper as its active IME (left by a crashed run, switched by another daemon, opened with `--no-test-ime`) was therefore denied the one channel that could have written its text: the helper's `am broadcast`, which is base64 and carries any Unicode. Cache-empty plus helper-active is exactly the state the previous commit set out to serve, and non-ASCII was the one input it still could not serve. Both entry points now read the input method first and take the broadcast route when it is the helper. The ASCII assertion moves to where the shell path is actually chosen, next to the ownership assertion it already sits beside, so it still fires — with the same error and the same precedence over `ime_capture` — for every caller that ends up chunking through `input text`. Regressions pin the state that was broken: with no cached IME state and the helper active, Unicode `type`/`fill` land in one broadcast, report the `test-ime` injection backend, and spawn no `input text` at all. The shell-path refusals get third-party-IME counterparts for both verbs; the `type` one had no adb stub and was reaching the host's real `adb` for a second per run. Also drops `RESULT.md`, the obsolete B1 worker handoff, from the tree. |
||
|
Michał Pierzchała
|
5e4a08f9a9 |
docs: define composable recorded fragments (#2018)
* docs: define composable recorded fragments * docs: guard composed fragment artifacts |
||
|
Michał Pierzchała
|
777c7af8cc |
fix: persist daemon-owned child process records (#2019)
* fix: record daemon-owned child processes (#1882) * fix: harden owned child cleanup identities |
||
|
Michał Pierzchała
|
07217c53fe |
fix(daemon): close web sessions on daemon shutdown instead of leaking the browser fleet (#2012)
* fix(daemon): close web sessions on daemon shutdown instead of leaking the browser fleet teardownSessionResources finalized recording, app-log, audio, and perf captures on daemon shutdown, but had no step for an open web session — unlike an ordinary `session close`, which dispatches a platform close for web. A SIGTERM on a daemon holding an open web session left the full agent-browser Chrome fleet (~15 processes) alive until agent-browser's own 5-minute idle timer fired, and a fresh daemon on the same state dir would not reap it either (startup orphan cleanup skips fleets with recent activity). Add a best-effort `web_browser` step to teardownSessionResources that tells agent-browser to close its session-scoped fleet, mirroring the recording step added in #1325. It runs after the other best-effort resource steps (recording, app-log, audio, perf), matching the ordering an ordinary `session close` already uses between its resource cleanup and its platform close. Since teardownSessionResources is shared by both the daemon-shutdown path and the expired-session reap path, both now close the browser immediately instead of leaving it to agent-browser's own idle timer. The per-session daemon-shutdown teardown budget is extended for a web session the same way it already is for an active recording, sized to (and tested against) agent-browser's own per-call CLI timeout, so the shutdown race doesn't give up on a slow close before agent-browser could have finished it. Extract isWebSession() as the single source of truth for "is this a web session", now shared by the new teardown step, the existing ordinary-close gate, and the web-provider request-routing gate, so the three cannot silently drift apart. Fixes #1868 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ot5wg8YUEyWRrphs8bMSJ * test(web): add a live daemon-shutdown lane proving zero owned Chrome processes The unit tests added for #1868 mock the agent-browser CLI call, so they prove teardownSessionResources dispatches `agent-browser close`, not that the managed Chrome fleet actually terminates. Add a second live-E2E scenario to smoke-web-platform.test.ts (same AGENT_DEVICE_WEB_E2E=1 gate as the existing smoke test) that opens a real managed web session, sends the daemon process a real SIGTERM, and asserts zero owned Chrome processes remain within a bounded settle window — kept well under agent-browser's 5-minute idle timer default (left unmodified, unlike the functional smoke test's shortened override) so a pass can only mean the daemon's shutdown teardown actively closed the browser, not that the idle timer coincidentally beat the poll deadline. Also runs the #1781 B1 daemon leak oracle against the same shutdown, wiring it to a web lane as the original issue asked for. I could not fully execute this test in the local sandbox: agent-browser's install step unconditionally fetches Chrome-for-Testing from googlechromelabs.github.io, which this sandbox's network policy blocks (confirmed via the proxy status endpoint, not assumed) even after installing Node 24 and pointing AGENT_BROWSER_EXECUTABLE_PATH at the sandbox's pre-installed Chromium. The repo's own CI already runs AGENT_DEVICE_WEB_E2E=1 with real network access (.github/workflows/ci.yml, "Execute live web smoke" step), which is where this new scenario will actually execute and get validated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ot5wg8YUEyWRrphs8bMSJ * fix(daemon): stop exporting stopSessionWebBrowser, its only caller is local CI's fallow dead-code gate (fallow audit, diff-scoped) flagged it as an unused export: unlike its siblings in this file, it has no second caller in the ordinary-close path (session close already reaches the browser through dispatchTargetedPlatformClose), so exporting it served no purpose. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ot5wg8YUEyWRrphs8bMSJ * fix(test): make the web-shutdown lane's idle-timer proof and cleanup real Two problems in the shutdown lane added for #1868: 1. createWebSmokeContext() unconditionally set AGENT_BROWSER_IDLE_TIMEOUT_MS to 30s for every caller, including the new shutdown scenario, whose own poll window is 45s. A reverted fix (no active browser close on shutdown) could still pass: agent-browser's own 30s idle timer would reap the fleet on its own well inside the 45s window, independent of whatever the daemon's teardown did or didn't do. The comment claiming production 5-minute behavior was simply false. Parameterize the context so only the functional smoke test opts into the shortened idle timeout; the shutdown lane now omits the override entirely, leaving the real 5-minute default in place (pinned by the existing `resolveAgentBrowserIdleTimeoutMs({})` case in agent-browser-lifecycle.test.ts) — comfortably past the 45s poll, so a pass can only mean the daemon actively closed the browser. 2. The scenario's `finally` only closed the fixture HTTP server. A failed assertion (including the exact failure mode this test exists to catch) left the daemon process and any still-running Chrome processes on the runner with nothing cleaning them up. Move cleanup authority fully into `finally`: stopProcessForTakeover on the daemon pid (a no-op if it already exited) and the same orphan sweep daemon startup runs for any leftover managed-browser processes, both best-effort and independent of how far the try block got, mirroring cleanupWebSmoke's AggregateError shape so a cleanup failure never swallows the assertion failure it ran alongside. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ot5wg8YUEyWRrphs8bMSJ * fix(test): make the web-shutdown lane's cleanup actually forceful Two more real gaps in the shutdown lane: 1. The finally block's browser-process cleanup called cleanupManagedAgentBrowserOrphans, which exists specifically to leave an actively-used fleet alone: it checks recent activity against AGENT_BROWSER_IDLE_TIMEOUT_MS and skips killing anything inside that window. Now that the previous commit restored a real (minutes-long) idle window for this lane, that guard would have suppressed the exact cleanup this test needs in the exact scenario it exists to catch — a reverted fix leaving Chrome alive would see this "cleanup" silently no-op rather than reap the leftover processes. Replaced it with forceKillManagedBrowserProcesses: the same listHostProcesses/summarizeAgentBrowserProcesses/expandProcessTree/ stopPidsWithEscalation primitives cleanupManagedAgentBrowserOrphans itself uses internally, called directly without its open-session or idle-activity skip guards, so this safety net reaps whatever the test's own fleet still owns regardless of how recently it was used. 2. The idle-timeout override the shutdown lane passes to createWebSmokeContext was an omitted env var, relying on knowledge of agent-browser's own default living elsewhere. Own the value directly instead: WEB_SHUTDOWN_IDLE_TIMEOUT_MS is computed as a fixed offset above the lane's own poll deadline, so neither a future change to the functional smoke test's override nor to agent-browser's default can silently invalidate the "idle timer can't have fired" claim this test's pass depends on. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011ot5wg8YUEyWRrphs8bMSJ --------- 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
|
a830ac8df2 |
feat: add Linux command evidence lane (#2017)
* feat: add Linux command evidence lane * fix: assert Linux find result shape * fix: read Linux find result envelope * fix: reset Linux calculator before diff * fix: release Linux session before reset * fix: guard Linux evidence session reset * fix: forward Linux evidence timeout * fix: tighten Linux evidence assertions * fix: preserve Linux replay session identity * fix: close Linux replay session before reset * fix: share Linux evidence daemon state * fix: keep Linux swipe evidence in bounds * fix: keep Linux artifact gap honest |
||
|
Michał Pierzchała
|
d97a628e38 |
fix(ci): make the two rg-based static checks actually run (#2006)
* fix(ci): make the two rg-based static checks actually run
ripgrep is never installed on ubuntu-latest, so both `rg` assertions in
the Lint & Format job failed with "command not found" (exit 127) on
every run. `if rg ...; then ... fi` cannot distinguish that from "no
matches" (exit 1) — both read as false, so each step silently passed
without its assertion ever executing. The DI-seams check had 7 live
violations it never reported.
Rewrite both against `grep`, which every runner ships, with match/
no-match/error exit codes handled explicitly so a broken scan fails
the lane instead of reading as a pass, plus a zero-tracked-files guard
so a renamed directory can't quietly go uncovered.
The DI-seam pattern also gets narrower to drop two classes of false
positive surfaced by actually running it: `typeof fetch` (fetchImpl?/
fetch? seams inject the one global with no module boundary vi.mock can
intercept; auth-session.ts/cloud-profile.ts/daemon-proxy.ts exercise
the seam directly in their unit tests, while CLI-level tests use
vi.stubGlobal('fetch', ...) where the seam isn't reachable — a
deliberate, exercised seam) and `typeof SOME_CONSTANT` in
SCREAMING_SNAKE_CASE (derives a literal union type from a constant,
e.g. interaction-touch-response.ts's dispatchPath field — not an
injectable seam at all).
Fixes #1976
* fix(ci): replace the DI-seam name-based allowlist with an explicit per-site one
Review on PR #2006 (#1976): the previous revision fixed the exit-code
handling but decided which `?: typeof X` matches to ban with a regex
that exempted matches by the *spelling* of the typeof target
(`typeof fetch` always passed, SCREAMING_SNAKE_CASE targets always
passed). That's a name-based semantic allowlist, not ownership: a new,
genuinely test-only `typeof fetch` seam anywhere in the tree would
have silently passed, while an equally legitimate seam under any
other name would still fail.
Add scripts/di-seams: a small, tested TypeScript checker that judges
each match against an explicit, typed, per-site allowlist
(scripts/di-seams/approved.ts) keyed by (file, field name, typeof
target) rather than by name. A triple is exempt only because it was
individually reviewed and named — never because of how it's spelled —
and the gate fails just as hard on a stale approval (one whose triple
no longer matches anything, e.g. after a rename) as on an unapproved
seam, so the list can't silently drift out of sync with the code it
describes.
Moves the DI-seams step in ci.yml to run after Setup toolchain (it's
no longer a toolchain-free text scan); the Swift trailing-comma check
stays where it was.
* fix(ci): register di-seams as a real gate and route it through the tmpdir wrapper
CI caught two things the local (dependency-free) run couldn't:
- oxfmt formatting on the two new files.
- scripts/node-test-tmpdir.test.ts's repo-wide audit: every package.json
script that invokes `node --test` directly must route through
scripts/node-test-tmpdir.ts, or a crash/timeout mid-run leaks its
scratch TMPDIR. check:di-seams now does.
- check:gate-manifest: a package.json script that runs `node --test`
must be covered by a registered CHECK_CATALOG gate, or the audit
reports the test suite as run by no lane. Registered 'di-seams' in
scripts/check-affected/{model,checks}.ts and wired the CI step
through run-gate like every other structural guard in this job,
instead of invoking pnpm directly.
Verified locally with node_modules installed: check:di-seams,
check:gate-manifest, check:gate-manifest:test, check:affected:test,
check:layering, check:fallow (scoped to the changed files), format,
lint, and typecheck all pass.
* fix(ci): close the multiline and duplicate-site gaps in the DI-seam scanner
Review round 2 on PR #2006 (#1976):
- findSeamMatches scanned line by line, so a declaration split across
lines (`field?:` on one line, `typeof X` on the next) was invisible.
Matching now runs against each file's whole source in one pass —
`\s` matches a real newline in JavaScript regexes with no extra flag
needed — with the line number derived from the match's character
offset.
- checkSeams keyed approval by (file, field, target) alone, so once
one occurrence of a triple was approved, any further occurrence of
that same triple anywhere in the file passed too. The key now
includes the line the match starts on, so an approval names one
specific declaration, not a recurring pattern. approved.ts expands
from 5 collapsed entries to the 7 exact sites this closes down to.
Added regression tests planting both gaps directly (a cross-line
declaration, and a second unreviewed fetchImpl?: typeof fetch at a
different line in an already-approved file) and verified both against
the real tree with injected violations, restored cleanly afterward.
Re-ran the full local gate suite (di-seams, gate-manifest, layering,
fallow, format, lint, typecheck) — all green.
* fix(ci): resync approved DI-seam line after merging main
Merging main (#2002) removed an unused import above the approved
dispatchPath?: typeof MAESTRO_COORDINATE_FALLBACK_PATH declaration in
interaction-touch-response.ts, shifting it from line 61 to line 60 —
exactly the location-specific-approval staleness the gate is designed
to catch, just triggered by an unrelated upstream edit rather than a
change in this PR. Updated the approved line to match.
* fix(ci): replace the DI-seam positional table with a code-local approval marker
Review round 3 on PR #2006 (#1976): CI proved the round-2 fix's core
assumption wrong within one push. Keying approval by (file, line,
field, target) made a line number the identity — an unrelated edit
anywhere earlier in a file shifts every approval below it, and that's
exactly what happened: merging main removed an unused import above
the approved dispatchPath declaration, and the gate rejected an
unchanged, already-reviewed line.
Detection is now AST-based (oxc-parser, the same tool
scripts/layering/*.ts already uses) instead of a source-text regex:
any `{ optional: true, typeAnnotation: TSTypeQuery }` node — a
property signature or a bare parameter — is a candidate, which finds
a multiline `field?:\n typeof X` declaration for free instead of
needing a special case for it.
Approval is a `// di-seam-approved: <reason>` comment immediately
above the declaration, matching this repo's own `//
fallow-ignore-next-line complexity` convention: the marker precedes
what it exempts. approved.ts (the external table) is deleted — there
is nothing left to keep in sync, since the approval travels with the
code it approves. A second, unmarked seam under the same field/target
elsewhere still fails; reordering unrelated code around an approved
declaration no longer touches it.
Added the marker to the 7 real approved sites (fetch-global
injection seams in auth-session.ts/cloud-profile.ts/daemon-proxy.ts;
the literal-type-derivation false positive in
interaction-touch-response.ts) and regression tests proving: a
cross-line declaration is still found, a second unmarked occurrence
of an approved field/target pair still fails, and an unrelated
insertion above an approved declaration no longer breaks it. Verified
against the real tree with an injected multi-line unrelated insertion
before an approved site — still green. Re-ran the full local gate
suite (di-seams, gate-manifest, layering, fallow, format, lint,
typecheck, auth-session unit tests) — all green.
* fix(ci): reject a di-seam-approved marker with no reason text
Review round 4 on PR #2006 (#1976): approvalReason() returned '' (not
null) for a bare `// di-seam-approved:` comment with nothing after
it, and checkSeams() only filtered out null, so an empty marker
silently approved a seam with zero justification — exactly the kind
of unreviewed bypass this gate exists to prevent.
approvalReason() now returns null when the joined reason text is
empty after trimming, so a bare or whitespace-only marker is treated
the same as no marker at all. Added tests for both the model-level
behavior and the end-to-end checkSeams() result, plus verified
against the real tree by injecting a bare-marker declaration and
confirming it's flagged, then restored cleanly.
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Michał Pierzchała
|
775eddd749 |
feat: session-scoped echo protection for parameterized recorded inputs (#2013)
* feat: session-scoped echo protection for parameterized recorded inputs Extends ADR 0017's fill-step-scoped guarantee to the whole recording session (#1398). After #1349, a later read-only action (`wait`, `is`, `get`) can independently observe and record an app-rendered echo of an already-parameterized `fill --record-as` value in its own result or target-v1 identity evidence, re-leaking the literal even though the originating fill was protected. - SessionState gains a small, ephemeral, never-serialized literal->placeholder registry populated only from explicit `--record-as` pairs, owned by session-action-recorder.ts. - Result/event payload fields get content-aware substring redaction (reusing the fill boundary's recursive scrub) for every literal registered so far in the session, longest-literal-first. - target-v1/targets-v1 identity evidence is never silently text-substituted while still claiming a trustworthy identity (replay compares against the live tree, which re-renders the real value). A landmark-mode (wait) echo is dropped to no annotation, exactly like #1349's existing identity-empty case, so an echoing landmark can no longer serve as an ADR 0016 destination guard. Action-mode evidence (get/is/mutating actions) redacts the label and downgrades verification to "unverifiable" instead, since ADR 0012/0016 forbid dropping required identity evidence. - Amends ADR 0017 (new mechanism), ADR 0012 (#1349/writer-invariant cross-references), and ADR 0016 (destination guard cross-reference). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RarRVX34ZW25TJejBZJ2Ui * fix: placeholder-safe single-pass multi-literal redaction Addresses review feedback on #2013: sequential single-literal replacement (register somethinglong -> ${ABC}, then ABC -> ${OTHER}) could rewrite a placeholder produced by an earlier pass, corrupting it to ${${OTHER}}. Replaces the per-pair sequential loop with one placeholder-safe left-to-right multi-literal pass (parameterizeAgainstLiteralMap): it never re-scans text it has already emitted, so no literal can be matched inside another pair's placeholder token in either direction. A registered literal is matched before checking for an existing placeholder token, so a value that itself happens to look like ${SOMETHING} is still redacted correctly. The scan uses a sticky regex instead of slicing per character, and literal pairs are sorted once per payload/evidence walk instead of once per string leaf. parameterizeRecordedFillPayload/parameterizeBackendOutput are generalized to take injected leaf-transform/carries callbacks so the single-pair fill-boundary path (with its existing whitespace-collapse behavior) and the new multi-pair session-wide path share one structural traversal. Adds regression coverage for both result payloads and action-mode target evidence, plus the placeholder-shaped-literal edge case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RarRVX34ZW25TJejBZJ2Ui * fix: unexport parameterizeAgainstLiteralMap (CI: fallow dead-code gate) Only used internally within this file (by parameterizeRecordedResultEcho and parameterizeTargetEvidenceEcho); the export had no consumer outside the module, which the fallow audit correctly flags as dead code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RarRVX34ZW25TJejBZJ2Ui --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
50f460cce4 |
refactor(snapshot): establish presentation ownership boundary (#2005)
* refactor(snapshot): establish presentation ownership boundary * docs: keep context glossary within budget * fix(snapshot): address presentation boundary review * test(snapshot): ratchet eager closure budgets |
||
|
Michał Pierzchała
|
4a3ccf67b4 |
test: close web platform command-coverage gaps from #1426 (#2011)
* test: close web platform command-coverage gaps from #1426 Closes #1900. The web coverage manifest carried 15 "known-gap" rows for commands with no web-specific evidence — some had never been checked against the web runtime, some just lacked a named test. Added dedicated tests and reclassified each row to command-contract: - boot, shutdown, install, reinstall, install-from-source, push, logs: new tests in packages/platform-web/src/runtime.test.ts prove the web runtime deliberately and permanently denies these operations (unsupported-platform-leaf), the same pattern already used for longPress/back/home/orientation/tvRemote/keyboard. - diff, press: new tests prove these commands share their exact runtime-execution plan with the already-live snapshot/click commands (snapshotRuntimePlanUses, pressRuntimeUses === clickRuntimeUses), so the admitted operation backing snapshot/click also backs them. - artifacts, events, batch, trace, replay: these commands have zero platform branching in their handlers; new tests dispatch each through its real production handler against a web-typed session/device to prove the existing generic code path works unchanged for web. - test: readReplayScriptMetadata deliberately drops `context platform=web` as a declarable value, so a scripted suite can never filter by --platform web. With no filter, discoverReplayTestEntries runs every discovered script unconditionally, so a new test proves an unfiltered `test` run executes correctly against a session already bound to a web device. Updates the manifest's classification-count gate to {capabilityDenial: 7, contract: 35, gap: 0, live: 12, total: 54} and replaces the now-vacuous "known gaps share one tracking issue" test with a planted-red proof that zero known-gap rows remain. Removes the now-dead WEB_COVERAGE_GAP_ISSUE constant and gap() helper. Live web-smoke scope is unchanged — no new commands were added to test/integration/smoke-web-platform.test.ts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SK6kvnNth6H7VxbbANi2xC * test: harden web coverage evidence from adversarial review An independent adversarial review of the previous commit found the `test` row's evidence was causally inert: the new test bound a web device to the session, but test-suite discovery (session-test-suite-command.ts) only ever reads flags.platform, never the session's device, so the test passed identically for any platform. Replace it with a test that proves the real, deterministic, web-specific behavior instead: `test --platform web` always reports zero matching scripts, because `readReplayScriptMetadata` drops `context platform=web` as an unsupported declared value and `ReplayTestPlatform` structurally excludes 'web' — so no `.ad` script, typed or untyped, can ever match that filter. Update the manifest row's assertion to state that limitation honestly instead of implying the command runs on web. Also: add a test pinning `press`'s runtime-execution plan to `click`'s (pressRuntimeUses === clickRuntimeUses, both descriptors reuse clickRuntimeUses) — the manifest's `press` row rested on that equality with nothing in the test suite that would catch it silently drifting. Fix a doc comment claiming appLogRuntimePlanUses spans all five app-log facts (it requires three; the other two are asserted as a reasonable superset, not because the plan needs them). Rework the manifest's top-of-file doc comment, which still described every contract row as "web-specific unit/provider evidence" after the previous commit added rows that instead prove a platform-agnostic code path or a structural limitation — state the three evidence shapes explicitly instead of retaining a sentence the new rows contradict. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SK6kvnNth6H7VxbbANi2xC * test: keep test's web row a known-gap per review @thymikee is right: the test row's evidence proves the opposite of what command-contract is supposed to certify. ReplayTestPlatform excludes web from the declared-platform filter entirely, so `test --platform web` can never select a script — that's real, tested, command-specific behavior, but it documents a limitation, not executable evidence that the command works on web. Reclassifying it as command-contract on the strength of that test would misrepresent what was actually shown. Reverts the test row to known-gap, restores WEB_COVERAGE_GAP_ISSUE and gap(), and updates the classification counts to {contract: 34, gap: 1} (14 of 15 gaps closed). #1900 stays open for this one row until a separate decision adds web replay-test support or an explicit denial. The regression test proving the exclusion (session-command-replay.test.ts) stays in place; it's just no longer cited as coverage evidence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SK6kvnNth6H7VxbbANi2xC --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
021fe2aa1d |
feat(maestro): support assertTrue phase 1 - literal/${VAR} truthiness (#1295) (#2010)
* feat(maestro): support assertTrue phase 1 - literal/${VAR} truthiness (#1295)
Adds the assertTrue command, scoped to literal values and bare ${VAR}
lookups per the #1292 lookup-only decision; JS expressions keep
failing loud at parse time with a runScript hint. Truthiness on a
looked-up value is evaluated against a pinned falsy-string table
("", "false", "0", "null", "undefined") since flow config/env/
runScript-output values are always stored as strings, rather than
native JS truthiness (which would treat "false" as truthy).
Wires assertTrue through the parser, interpreter, optional/warning
composition, and the layer-1 conformance oracle, narrowing the
067_assertTrue_pass divergence to the JS-expression case and removing
the now-satisfied 076_optional_assertion entry. Also materializes
scrollUntilVisible's default direction in the conformance canonical
projection, a latent gap only exposed once 076 could fully compare.
* fix(maestro): correct assertTrue truthiness claim in CLI help/docs
The support-matrix text said assertTrue is "evaluated with JS
truthiness", but the engine actually uses a pinned falsy-string table
("", "false", "0", "null", "undefined") since looked-up values always
arrive as strings — native JS truthiness would treat "false" as
truthy. Spell out the actual rule instead of the misleading claim.
* fix(maestro): fix oxfmt quote-style violation in expected-divergence.ts
CI's format gate failed on a single-quoted string containing an
apostrophe; oxfmt prefers double quotes there.
* fix(maestro): bump eager-closure-budget pin for the new truthiness module
engine-truthiness.ts is a genuinely new module on the core interpreter
path (assertTrue is dispatched unconditionally by
replay-plan-step-execution.ts), so packages/maestro/src/index.ts now
eagerly evaluates 105 modules instead of 104 — a deliberate growth,
not a laziness regression.
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Michał Pierzchała
|
942a310fb3 |
perf(selectors): collapse the double tree scan for disambiguate/fail-closed rows (#2009)
resolveSelectorChainWithPolicy ran listSelectorChainMatches (one full scan per alternative) and then resolveSelectorChain (another full scan per alternative) for the disambiguate/fail-closed rows readText/readUnique use — the same defect class #1690 removed from replay's resolveRecordedTarget. resolveSelectorChainDomain now tracks the first alternative that matched anything (`firstMatch`) unconditionally, in the same pass that already decides the winning resolution, so resolveSelectorChainWithPolicy needs only one call for these rows. matchedNodes keeps naming the first alternative that matched (not the winner) when they differ, preserving the existing contract wait's landmark check and the ambiguous outcome rely on. Also fixes analyzeSelectorMatches's lazy isVisible: it now builds the viewport-root rect list once per alternative (via the newly extracted collectViewportRects) alongside the existing lazily-built byIndex map, instead of isNodeVisibleOnScreen re-deriving it on every ambiguous candidate. Closes #1970 Claude-Session: https://claude.ai/code/session_01YJoiggu7utUNDBmdzSBK2h Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
6e532003ad |
test(daemon): harden the timed-out wait decoration test against CI load (#2008)
* test(daemon): drive the timed-out wait decoration test on a fake clock
The 1ms wait timeout raced the real clock: under CI load the first capture
could take longer than 1ms to resolve, so runWithinWaitDeadline classified
the poll capture-stalled/capture-truncated instead of a completed
target-absent poll, and maybeWaitTimeoutSurfaceResponse skipped the
"Current surface" decoration outright — causing the intermittent CI Coverage
failure on PR #2000 (run 32752619090, head
|
||
|
Michał Pierzchała
|
dbc4f2f955 |
chore(test): start the subprocess-stub kill-criterion experiment (#1823) (#2007)
Deletes the serialized `subprocess-stub` Vitest project and drops SUBPROCESS_STUB_TESTS from unit-core's exclude, so its two real spawners (client-metro.test.ts, harness.test.ts — corpus-replay.test.ts already left for fuzz-worker in #1994) run un-serialized in the default forks pool per #1823's own kill criterion. Revert if a timeout-shaped failure shows up before 20 consecutive CI runs pass clean. The files stay excluded from the mutation lane (SERIALIZED_TESTS): that exclusion is about mutant-rerun cost, independent of Vitest project structure. Updated the comments/docs/scripts that described the old project by name so none of them assert a project that no longer exists. Claude-Session: https://claude.ai/code/session_015YPgKE1xmjdqh7T1q987DA Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
893ce4b866 |
fix(ci): repair nightly XCTest and conformance lanes (#1989)
* fix(ci): repair nightly XCTest and conformance lanes * fix(ci): harden nightly failure classification * fix(ci): stabilize macOS replay cleanup * fix(ci): close nightly review gaps * fix(ci): classify device claims as infrastructure |
||
|
Michał Pierzchała
|
054dcd4ea4 |
refactor(snapshot): type the acquisition producer beside the platform channel (#2000)
* refactor(snapshot): type the acquisition producer beside the platform channel Three producers with different guarantees share the backend: 'xctest' stamp (Apple runner, Appium page-source, limrun element trees), and the 'android' channel conflates the local uiautomator path with Appium trees the same way. Add SnapshotProducer as a required field on SnapshotResult so the compiler enumerates every producer, and carry it into SnapshotState. Types-before-semantics step for #1983; no consumer behavior changes. * refactor(snapshot): make provenance one kernel-owned pair table Review follow-up on #2000: backend and producer were independent unions, so cross-channel pairs type-checked. SnapshotProvenance now owns the legal channel<->producer pairs; SnapshotBackend is its projection, SnapshotResult embeds the strict pair, SnapshotState embeds the optional-producer variant, and buildSnapshotState carries the pair through a narrowing helper so the fields never decorrelate. Negative type-level regression pins that invalid pairs cannot compile. |
||
|
Michał Pierzchała
|
dd5dd4385d |
refactor: make interaction response cleanup platform-neutral (#2002)
* refactor: make interaction response cleanup platform-neutral * test: cover commandless interaction cleanup |
||
|
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
|
104fe75248 |
fix(ci): run the fuzz corpus replay outside the coverage lane (#1994)
The Coverage job intermittently ends with no failing test and one file's
results missing:
Test Files 1070 passed (1071)
Errors 1 error
Error: [vitest-pool]: Worker forks emitted error.
Caused by: Error: Worker exited unexpectedly
This is shape (B) of #1824 — the half #1854 did not fix. Scanning every
failed Coverage job across the 120 CI runs since #1854 merged finds the
signature five times, and the vanished file is
scripts/fuzz/corpus-replay.test.ts all five (six for six with #1866's
occurrence): 23% of Coverage failures in that window, ~4% of all CI runs.
The ~40s gap before the error is coverage report generation, not test
time — the pool surfaces its AggregateError only once every task settles.
Control, from a green attempt of the same run: the file passes in 3152ms
at 09:37:35.9 and the summary prints at 09:38:12.5. So the file is not
slow in CI, nothing else is in flight when it dies, and neither a missed
per-case budget nor STARTUP_BUDGET_MS is implicated. Partial test counts
(3/11 and 9/11 reported) place the death mid-file, inside runCases.
So the corpus replay gets its own serialized project that the coverage
run skips, and a second uninstrumented Vitest invocation in
`test:coverage:ci` runs it, keeping the tests on every PR. Measured
against two full runs, this costs zero coverage: the cases execute in
worker threads, a separate isolate the fork's inspector never
instruments, so the lines reported are identical with and without it.
Membership is by demonstrated failure, not by a property of the code:
`session-replay-runtime-maestro.test.ts` also constructs a
node:worker_threads Worker and stays in unit-core, instrumented and
green, so "nests a Worker" is explicitly not the criterion.
The second leg goes through `test:fuzz-worker`, which blanks
AGENT_DEVICE_COVERAGE_SHARD and AGENT_DEVICE_COVERAGE_MERGE. ci.yml sets
those as job-level env over a single `gate: unit-ci` step, so both legs
would otherwise inherit them and the shard would die: Vitest refuses
`--shard=1/2` over this one-file project, and the blob reporter
overwrites the instrumented shard's report on its way out. Verified on
the merged tree — shard 1/2 (549 files), shard 2/2 (548), and the merge
job (1097 files, 90.38% lines) all pass, and the leg still fails without
the blanking.
Refs #1824
|
||
|
Michał Pierzchała
|
02d548dfc9 |
ci: consolidate CI workflow from 15 jobs to 8 (#1996)
* ci: consolidate CI workflow from 15 jobs to 8 Merge single-gate ubuntu jobs into grouped jobs sharing one checkout and install: Lint & Format (plus the static text assertions), Repo Guards (layering/selector/wiring/maestro/mcp-metadata), Compatibility & Provenance (shared fetch-depth: 0 checkout), Typecheck & Package, and Integration Tests (absorbs the web smoke with step-scoped env). Every gate remains an independently named run-gate step; the gate manifest derives lane ownership structurally. Drop the Bun setup from FreeRange: @chenglou/freerange's bin is a plain Node script. It stays GitHub-owned; only the runtime requirement is retired. * ci: fold FreeRange into Repo Guards and skip no-op fixture release jobs FreeRange runs on plain Node now, so its gate joins Repo Guards as the last step instead of occupying its own worker for the slowest guard. The fixture release matrix filters to entries that will actually build, so a cached-fingerprint PR starts zero release runners. * ci: fold host XCTests into the macOS smoke lane and shard Coverage The macOS lane now builds one unit-test-flagged runner bundle that both the host XCTest run and the replay smoke consume, so the host lane no longer occupies its own macos-26 runner behind a separate queue. The host lane's file moves with it, and check:xctest-selection follows. Coverage shards across two runners via blob reports and merges them on a report job that evaluates thresholds once over the full suite and produces every coverage artifact. The tmpdir leak check runs per shard, since a leak lands on whichever runner executed the file. * ci: drop local shard-smoke artifacts from tracking * ci: enforce coverage thresholds only on the merged run A shard evaluates its own half-suite coverage, so the global gate fired per shard. Shards now report without gating; Coverage Report keeps the real thresholds over the full merged suite. * ci: include hidden files when uploading coverage blobs |
||
|
Michał Pierzchała
|
6e45d88b94 | fix: stop replay retries after cleanup failures (#2001) | ||
|
Michał Pierzchała
|
d713988c5a |
docs(agents): simplify testing and pull-request guidance wording (#1997)
* refactor(lint): replace the facade import scan with a lint rule The surviving half of `contracts-entry-closure.test.ts` walked ~490 candidate files and parsed each one to assert that nothing value-imports the two wide contracts facades. `eslint/no-restricted-imports` already states exactly that, and `allowTypeImports` already draws the one distinction that made the walker seem necessary: `import type` is erased, so it stays legal. Verified rather than assumed, because the override semantics are not additive: a same-rule override REPLACES the parent, so a top-level rule would have been silently dropped for `src/**`, and the existing `"off"` entry for `exec.ts` and the test tree would have exempted the files that carried most of the cost #1959 removed. So the paths are added per zone, and the blanket `"off"` becomes a facade-only config that keeps the `node:child_process` exemption it existed for. Planted red in all three zones — `src/core/capabilities.ts`, a `src/__tests__` file, and `packages/capture-kit/src` — each flagged, while a type-only import in the same probe file was not. A first probe read as a pass because the sed that built it produced a type-only import; the zone was re-probed with a real value import rather than trusting the green. Misconfiguration fails loudly, which is why this is safe to rely on: a typo'd rule name makes oxlint exit 1 with "Rule not found in plugin", not pass silently (the failure mode #1976 records for the `rg` assertions). What a linter cannot replace, and stays: the eager-closure budgets. Those are a transitive-weight property — a module already imported grows an import, and the cost arrives without any single file's import list changing. Per-file rules cannot see that, and `no-restricted-imports` can only ban specifiers named in advance, which is precisely what #1950/#1956/#1959 could not have named. * docs(agents): simplify testing and pull-request guidance wording testing.md sat 15 bytes under the 10k per-doc check:agent-guidance cap. Rewrite both docs in shorter, plainer sentences without dropping any fact, threshold, or identifier (backtick-identifier sets verified unchanged against the previous revision). Also fix testing.md's gate catalog sentence being separated from its code block and the missing blank line before pull-requests.md's Reviewing section. |
||
|
Michał Pierzchała
|
66458c7916 |
perf(selectors): resolve a recorded replay target in one matching pass (#1988)
* perf(selectors): resolve a recorded replay target in one matching pass resolveRecordedTarget ran two full scans of the snapshot tree per call: resolveSelectorChain visited every node to pick a winner, then the caller either re-filtered every node with the winning selector or handed the chain to listSelectorChainMatches to re-derive the same domain. resolveSelectorChainDomain returns the matched-node set the deciding pass already collected — the winning alternative's when one resolved, the first matching alternative's when none did, which is the set listSelectorChainMatches reports. resolveSelectorChain now delegates to it, so its shape and the published ./ast surface are unchanged, and listSelectorChainMatches stays exported for its three other callers. Winner, matchedNodes, matchCount, disambiguation disclosure, and the ambiguous-vs-no-match classification are unchanged. Observed red first: the new traversal-count regression reported 1 redundant filter scan on both the resolved and the unresolved leg. * test(selectors): count the full array-scan surface in the replay traversal gate Review F1: `observeTreeTraversals` counted only `Symbol.iterator`, `filter` and `map`, so a regression that reintroduced a whole-tree pass through `flatMap`/`forEach`/`reduce`/`some` kept the assertion green. Widen it to the whole scan surface and pin the true counts. That exposes eight `flatMap` scans on the ambiguous leg — four whole-tree viewport-rect lookups per ambiguous candidate, charged by `isNodeVisibleOnScreen` with no precomputed viewport rects. They predate this branch and are left alone here; pinning them keeps the number from growing unnoticed. Rename the test to what it proves: no SECOND matching pass per alternative, not "reads the tree once". The review's F4 note on `ActionableTouchTopology.viewportRootRects` moved to the find-ranking branch this one stacks on, since it documents that branch's type. * test(selectors): count find/every/indexOf-class scans in the traversal gate The replay traversal counter only watched filter/map/flatMap/forEach/reduce/ some/iterator, so a redundant full-tree scan expressed via find, every, findIndex/findLast(Index), includes, indexOf, or reduceRight stayed green. Expand SCAN_METHODS to the full scan surface; planted-red demonstrated by injecting a single nodes.find() into resolveRecordedTarget (gate fails with find: 1) and reverting. |
||
|
Michał Pierzchała
|
6c4b740865 |
fix(test): drain session-event-log writes before rmSync cleanup (#1999)
The save-script transport tests' afterEach removed the per-test temp root while a fire-and-forget session-event-log append (queued by every request the tests send) could still be in flight. Under coverage-shard load the append re-created the session dir between rmSync's unlink sweep and its rmdir, failing cleanup with ENOTEMPTY. Await flushSessionEventLogWrites() before removing the roots. Closes #1998 |
||
|
Michał Pierzchała
|
bcca714a07 | refactor: move gesture family to platform runtime (#1952) | ||
|
Michał Pierzchała
|
759f175332 |
refactor: move Wave 5 touch commands to platform runtime (#1987)
* refactor: move touch commands to platform runtime * fix: require direct selector touch binding * fix: address touch runtime review * fix: classify maestro direct click guarantee * fix: distinguish Maestro direct selector dispatch |
||
|
Michał Pierzchała
|
3e06884fcd |
fix(ios): verify fill's synthesized-replacement route before reporting success (#1995)
* fix(ios): verify fill's synthesized-replacement route before reporting success
The channel-penalized fill route (runSynthesizedReplacementRoute, taken when
the XCTest accessibility channel is already penalized under load) posted the
synthesized keystrokes and returned ok:true without ever reading the field
back, so a dropped or still-in-flight character was indistinguishable from
success. This is the same class of bug already fixed for bare `type`
(#1676/#1924), but that fix never covered this fill-only route.
Reusing type's append-mode commit-wait verbatim would have been wrong: its
"observed value isn't a prefix of expected -> trust the app" rule exists to
tolerate legitimate transforms (autocomplete, formatters) during append, but
it also waves through a dropped-middle-character corruption, since a value
with a hole in it is neither a matching prefix nor an exact match. Verified
against the real corruption strings ("Ada Lovelace" -> "Avelace", "ada@example"
-> "aexample") that the old rule would classify both as "trust it" and never
fail. Added a separate replacement-mode outcome function with no such
escape hatch, consistent with how isRepairableTextEntryMismatch already
treats every .replacement-mode mismatch as failing/repairable unconditionally.
Verified on a real iOS Simulator via xcodebuild test-without-building, not
just a build: the new regression test proves the old model accepts both
corruption strings while the new one correctly reports commit-not-observed,
and the full pre-existing append/type test suite passes unchanged.
* fix(ios): restore labeled observe-closure call sites for the redaction guard
The previous shared-plumbing refactor passed each route's outcome function
as a stored closure parameter, which erases Swift argument labels at the
call site. That broke the CI "Coverage" check's static content-redaction
test (apple-runner-log-redaction.test.ts), which locates the commit wait's
observe closure by its literal `observe: {` label to verify it only logs
polled field content through the value-free logCommitCadence boundary,
never a raw NSLog.
Restructured so the shared placeholder/deadline/observe/pacing ingredients
are still factored into one place, but each of the two public entry points
(append/type, replacement/fill) now calls its own named outcome function
directly with real argument labels, restoring the labeled closure shape the
guard depends on. Verified locally: the TS redaction test passes, and the
full on-device unit test set (19 tests, iOS Simulator) still passes with
zero regressions.
|
||
|
Michał Pierzchała
|
c34d27e3f9 |
refactor(lint): retire the facade closure test for a lint rule plus budget rows (#1990)
`contracts-entry-closure.test.ts` (from #1959) carried two assertions. Neither replacement subsumes it alone; together they do, on two different axes. Its whole-tree scan parsed ~490 candidate files to prove nothing value-imports the two wide facades. `eslint/no-restricted-imports` states exactly that, and `allowTypeImports` draws the one distinction that made a custom walker look necessary: `import type` is erased, so it stays legal. The rule covers every file rather than the four hubs the deleted test named, and it reaches dynamic imports too. Its hub pin checked that four named hubs never reach either facade. The lint rule forbids the import edge that was the only way that could happen, and #1960's rows add an axis the old test never had: closure SIZE drift. That cardinality check is deliberately not described here as strictly stronger, because it is not — an equal-size graph substitution leaves the count intact and passes. Size drift and forbidden edges are different properties, and the two gates own one each. The override semantics are not additive, so the rule was verified per zone rather than assumed. A same-rule override REPLACES the parent, so a top-level rule would have been silently dropped for `src/**`; and the existing blanket `"off"` for `exec.ts` and the test tree would have exempted the files that carried most of the cost #1959 removed. The paths are therefore added per zone, and that `"off"` becomes a facade-only config that keeps the `node:child_process` exemption it existed for. Planted red in all three zones — `src/core/capabilities.ts`, a `src/__tests__` file, and `packages/capture-kit/src` — each flagged, while a type-only import in the same probe file was not. The first packages probe read as a pass because the sed that built it produced a type-only import; that zone was re-probed with a real value import rather than trusting the green. Relying on config is safe here because misconfiguration fails loudly: a typo'd rule name makes oxlint exit 1 with "Rule not found in plugin", rather than pass silently the way the `rg` assertions in #1976 did. The budgets stay for what no linter can express: a transitive weight property, where a module already imported grows an import and the cost arrives without any single file's import list changing. Per-file rules cannot see that, and `no-restricted-imports` can only ban specifiers named in advance — exactly what #1950/#1956/#1959 could not have named. |
||
|
Michał Pierzchała
|
cc521decb8 |
test(structure): per-package eager-closure budgets — the ADR-0019 loading-shape probe (#1739) (#1965)
* test(structure): per-package eager-closure budgets (#1960) ADR-0019 requires platform-package façades to stay implementation-lazy and is explicit that a startup threshold alone is not a substitute for preserving the loading shape. #1950 built the AST-level walker (eager-import-closure.fixtures.ts) and proved the planted-red procedure on one file (session-teardown.ts's android-helper denylist); this generalizes it into a data-driven budget table so any workspace-package façade -- or a designated hub module -- can get an eager-closure ceiling without a bespoke test. Seeds a budget for every packages/*/src/facades/*.ts file (discovered the same way package-boundaries.test.ts discovers façades, not hand-listed) from its measured current closure size, plus a platform-implementation denylist for façades whose contract is implementation-neutral vocabulary. Also demonstrates the mechanism on two designated hub modules (cli.ts, session-teardown.ts) alongside their existing, more specific ad hoc pins. Closes #1960 * test(structure): derive facade roots from manifests, add edge chains, reseed tight Review findings on #1965: 1. Discovery scanned only `packages/*/src/facades/*.ts`, which omits every package that publishes its entry surface straight from the manifest — including all six `packages/platform-*/src/index.ts` façades, the exact subject of ADR-0019's implementation-laziness rule. Discovery now derives from `readWorkspacePackages(...).exportTargets` and then adds `/src/facades/` files, reusing the R11 helper rather than reimplementing it so the two gates cannot disagree about what an entry surface is. The table grows from 14 façades + 2 hubs to 95 entry surfaces + 8 hubs. 2. Budgets carried a few files of slack each. They are now exact ratchets with no headroom, matching how the repo pins R9/R10 and test-file size: growth is allowed, it just has to be a visible number change in the diff of the PR that causes it. Every budget is reseeded from post-#1969 measurement. 3. Violations printed a flat sorted set, which named the offender but not the route. `eagerClosureGraphOf` records each file's discoverer, so failures now print the transitive chain entry -> ... -> offender. `eagerClosureOf` keeps its contract and is expressed in terms of the new walk; per-file edges are memoized, which also cuts the existing pins' runtime (cli closure test 2336ms -> ~550ms). The platform-package façades evaluate exactly one module each — themselves — so their budget of 1 is the tightest statement of "metadata-eager, implementation-lazy" the walker can make. * test(structure): make the eager-closure pins exact, bounded, and single-owner Second review pass on #1965 found four holes, two of which were places the PR text claimed a property the code did not have. 1. Rows were documented as exact ratchets but asserted with `<=`, so a shrink silently became headroom a later regression could grow back into. The comparison is now equality, in a pure `classifyBudget` with a separate message for each direction ("lower its pin to N in this PR so the ratchet keeps the gain"), matching test-file-size-ratchet.ts and the R9/R10 pins. 2. An over-pin failure printed a chain per evaluated module — 361 of them for src/cli.ts. It now prints a bounded attribution: the entry's heaviest direct edges (capped at 4) with a couple of representative deep routes each, ranked so a newly added import sorts first. The comment states plainly that this attributes by shortest import route and does NOT diff against a recorded baseline; naming a true delta would mean checking in ~1,500 module paths and rewriting them on every contracts refactor. 3. Discovery reimplemented a one-level `src/facades` scan while canonical R11 discovery is recursive, so a nested façade file could be covered by R11 and silently missing here. `facadeEntryFiles` is now a single exported owner in package-boundaries.ts that both R11's façade gate and this table consume. 4. Rows were converted to Sets before any uniqueness check, so a duplicate was unobservable. The table is now two `Record<string, number>` literals keyed by path, making an in-record duplicate a TypeScript error (ts1117); the only remaining case — one path in both records — is asserted on the array. Each of the four holes gets a test that fails when the rule is broken, since a tree that happens to satisfy its pins cannot distinguish a correct rule from a vacuous one. Writing those found a real bug in the duplicate check itself (`Set.add` returns the Set, so the filter never matched). Pins reseeded on |
||
|
Michał Pierzchała
|
8d280a2081 |
refactor(request): move the device-inventory context out of core (#1993)
`device-inventory-context.ts` is an AsyncLocalStorage holder: it imports only `node:async_hooks`, kernel errors, and contracts types, and nothing from `core`. `daemon-modularity.ts` already describes `src/request/` as "request-global daemon plumbing (progress sinks, cancellation, AsyncLocalStorage)", and `request/progress.ts` is the same shape — a `withX`/accessor pair over one store. Living in `core` (rank 2) made the platform-runtime composition root reach up to rank 2 for `listLocalDeviceInventory`. Injecting the lookup would not have fixed that: the only caller, `platform-runtime-operation-host.ts`, is in the same zone, so the edge would move rather than disappear. Relocating the module does remove it, and every other consumer — daemon (rank 4) and `core/dispatch-resolve.ts` (rank 2) — now reaches down instead of sideways or up. Pure rename; the module body is unchanged and 14 import specifiers are retargeted. R17's `DEVICES_INVENTORY_IMPORT_SOURCES` pin follows the module: the rule still asserts the devices handler imports the neutral gateway, does not shadow it, and calls it. This clears the last composition-root upward value edge that was not deliberate. Confirmed with `pnpm depgraph`: those edges drop from 3 to 2, and the two that remain are the documented ones — `provider-limrun-runtime.ts` (an in-file comment marks it a deliberate static seam) and `runtime.ts -> commands/index.ts` (`bindCommands` is the aggregate binder). Cycle, back-edge and R6 counts unchanged. Claude-Session: https://claude.ai/code/session_01WWLBCBDBpdR8z1aCmDerXS Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
777ded9d75 |
refactor(test): fold the last two coverage-summary copies into the shared builder (#1992)
tvOS (#1919) and macOS (#1922) each carried a private copy of buildCoverageClassificationSummary; Android, web, and Linux already import the shared one from test/integration/support/coverage-classification.ts. Fold the remaining two onto it and drop their structurally identical local summary types, completing the factoring that #1426's ground rule triggered at the fourth copy. Refs #1426 |
||
|
Michał Pierzchała
|
296447707e |
refactor: migrate back/home/orientation/tv-remote/keyboard to the request-bound device runtime (#1955)
* refactor: migrate back/home/orientation/tv-remote/keyboard to the request-bound device runtime Continues the ADR 0019 platform-runtime migration (Wave 5 generic leaves): five generic-route commands move off dispatchKnownCommand/Interactor legacy dispatch onto fact-owned admission, one bind per handler. keyboard uses the R35 action-selected single-bind pattern (status/dismiss/enter each admit and bind independently). All 8 owner runtime packages gained fact-cell tests for the new operations; six smoke-coverage integration oracles and nine daemon/capability unit test files were updated for the retired capability- catalog admission these commands no longer carry. * refactor: extract shared interactor-resolution prelude in keyboard-runtime bindKeyboardStatus/Dismiss/Enter repeated the same signal-check + resolveInteractor call; factor it into resolveKeyboardInteractor so each binder is a two-line call instead of a six-line copy. No behavior change — the three contract-module mutants planted earlier in review still kill on this shape. * fix: refuse watchOS admission for back/home/orientation/keyboard; pin tv-remote non-TV parity P1: watchOS has no constructible Apple interactor (XCUITest cannot drive its UI, ADR-0009), matching the existing captureScreenshot/captureSnapshot/readTextAtPoint/findSelector pattern in this same file. appleBackFact/appleHomeFact/appleMobileInputEligible admitted every Apple OS but tvOS/macOS, wrongly including watchOS. Facts now refuse watchOS explicitly for back, home, orientation, and keyboard dismiss/enter, with a fact-cell test asserting no binding for every one of them. P2: verified the daemon's generic-route capability gate already reproduced the retired per-platform tv-remote hint text (message stays the generic "<command> is not supported on this device", hint carries the owner-specific text) for every device that could reach dispatch in the old system -- the retired handleTvRemoteCommand's own "supported only on TV targets" check was unreachable there and only exercised by a test calling dispatchCommand directly. Added a daemon-level test pinning the exact iOS and Android-mobile hint strings to make that parity explicit instead of implicit. Also fixes a fallow complexity finding the P1 test edit introduced by splitting the fact-cell assertions into five small named helpers instead of one large function. * fix: stop orientation-runtime.test.ts's router-join test from hitting real adb Root-caused the CI-only Coverage failure (unreproducible locally in isolation, reproducible 2/2 in the full CI run): every generic-route leaf this migration touches carries `androidBlockingDialogGuard: true`, and `dispatchGenericCommand` calls `ensureNoAndroidBlockingDialogReady` unconditionally for any `platform: 'android'` session reaching the real request router -- regardless of whether admission is fact-based or capability-based. That check calls `getAndroidBlockingDialogFocus`, which shells out to the real `adb` binary. orientation-runtime.test.ts's "request router joins..." test used a synthetic `platform: 'android'` device through `createRequestHandler` (the real router), without stubbing the platform ADB layer -- only the runtime gateway was mocked. On a host with a real `adb` binary (my machine) the subprocess fails fast and `allowFailure` tolerates it, costing ~800ms-1.1s but still succeeding. On a host with no `adb` binary at all (CI's Coverage job, a plain unit-test lane with no Android SDK) the spawn itself throws, which isn't something `allowFailure` catches, producing exactly the observed `ok: false` unsupported-operation response. back/home/tv-remote's equivalent router-join tests already use Apple/Vega devices, so they never reached this path. Switched orientation's fixture to match -- Apple, since the fixture's facts/execution are fully synthetic and platform-agnostic regardless. Also: renamed the widely-shared 'emulator-5554'/'ios-simulator' device-id literals in back/orientation/tv-remote/keyboard-runtime.test.ts to file-scoped ids. Device claims for a `local-family` owner binding hit the real on-disk `require-owner` claim file (keyed only by canonical device id), and 27+ pre-existing test files already share 'emulator-5554'; this migration added three more consumers of it under a `require-owner` policy that reaches real admission, which was worth eliminating as a source of doubt even though it wasn't the actual root cause here. * refactor: extract navigation/keyboard concepts into sibling modules; test the real Android dialog-guard path packages/platform-apple/src/runtime.ts and packages/provider-limrun/src/app-log-runtime.ts grew past the repo's 500-line extraction threshold. Move the new back/home/orientation/ tv-remote/keyboard facts and bindings into packages/platform-apple/src/navigation/runtime.ts (new sibling module, matching deployment/runtime.ts's existing pattern), and the new keyboard facts/bindings for limrun into the existing packages/provider-limrun/src/interaction-operations.ts (which already held the sibling navigation logic). Also fix orientation-runtime.test.ts's router-join test: it previously swapped its device fixture from Android to Apple to dodge the real adb-backed blocking-dialog guard, which masked the Android route that was actually failing in CI. Keep the Android fixture and stub getAndroidBlockingDialogFocus instead, the same seam request-router-android-modal.test.ts already uses. * refactor: adopt granular contracts subpaths for back/home/orientation/tv-remote/keyboard Following main's #1969 (facade granularization), give each of this branch's five new contract modules their own package.json entry subpath and move every value-importer (owner runtime packages, the daemon binders, and their tests) off the wide @agent-device/contracts/platform facade onto the specific module that owns the symbol — the same convention #1969 established for the rest of the vocabulary. Keeps this migration's files out of the contracts-entry-closure gate and out of the eager-evaluation cost #1969 measured for the daemon's permanent hubs (registry.ts, dispatch.ts). * refactor: shared navigation/keyboard binder table; dedupe keyboard admission; drop restated types Addresses the review's finding 1 (seven per-owner copies of the same "fact-keyed table of interactor binders" pattern) by extracting bindAdmittedLocalInteractorOperations/bindAdmittedProviderInteractorOperations into packages/contracts/src/interactor-operation-catalog.ts. Each owner now requests the subset of back/home/setOrientation/tvRemote/keyboard{Status, Dismiss,Enter} it admits, instead of hand-writing `facts.operations.<key>.available ? bind…(resolver) : {}` per operation. Applied across all seven call sites (apple, android, harmonyos, vega, linux, webdriver, limrun) and collapsed limrun's two separate bind functions (navigation, keyboard) into one shared call. Finding 3 (resolveBoundKeyboardRuntime copy-pastes admit-then-wrap three times): extracted a local admitKeyboardAction<...> helper mirroring resolveBoundGenericRuntime's admit-then-defer shape, so the three action branches (status/dismiss/enter) share one admission path. Finding 6 (execute* helpers hand-restate a contract that can drift): back/ home/orientation/tv-remote/keyboard's execute functions are now typed off `BoundDeviceRuntime<typeof xRuntimeUse>` (derived from the actual bind-use value) instead of a hand-written `Readonly<{ operations: Readonly<{...}> }>` shape. Also fixed provider-limrun's `RuntimeOperationUnavailability | { available: true }` restating RuntimeOperationFact by hand — folded away entirely once the bind functions it typed were removed. Finding 7 (naming/placement): platform-apple/runtime.ts's misleadingly-named `captureOperations` bucket (held deployment/network/recording/find, not just capture) collapsed into one flat `operations` object now that the navigation bucket is a single function call instead of six ternaries. Exported RuntimeAdmissionRequest from runtime-admission.ts (needed by the new keyboard admission helper). Added packages/contracts/src/ interactor-operation-catalog.test.ts for the new shared binder table. pnpm typecheck, check:fallow, check:layering, and the full unit-core suite (1010 files / 7513 tests, one known contention-flake excluded) are green. * refactor: split generic-mutating command traits from the legacy dispatch pair Addresses the review's finding 5: GENERIC_MUTATING_LINUX_DEVICE_COMMAND_TRAITS bundled two orthogonal things (daemon/recording traits, and the legacy capability+dispatch pair migration strips), forcing every migrated descriptor to hand-expand the constant minus two fields plus an explanatory comment. Split into GENERIC_MUTATING_COMMAND_TRAITS (the shared daemon/recording traits) and LEGACY_LINUX_DEVICE_EXECUTION (the dispatch/capability pair). back/home/orientation/tv-remote (this migration) and focus (an earlier one, same pattern, previously a stale reference to the retired constant name) now spread the trait constant directly instead of hand-expanding it; the still-legacy `scroll` descriptor spreads both pieces, equivalent to the retired constant. pnpm typecheck, check:fallow, check:layering, and the registry/daemon test suites are green. * refactor: table-ify packages/contracts/src/keyboard-runtime.ts's three-way duplication Finding 3's second half: the three bindKeyboardX functions and six bindLocal/ProviderKeyboardXInteractor entry points differed only by method name and label string. Replaced with one generic bindKeyboardAction<Key> dispatching off the operation key (interactor[key], resolved from a small label table) plus two shared local/provider dispatch helpers the six named exports each call with their own key — collapsing three copies of the bind logic into one and six near-duplicate entry-point bodies into one line each, while keeping every exported name and type signature unchanged. pnpm typecheck, check:fallow, check:layering, and pnpm check:affected --run are green. * refactor: parameterize runSessionOrSelectorDispatch with an execute strategy Addresses the review's finding 2: handleKeyboardCommand re-implemented runSessionOrSelectorDispatch's orchestration step for step (session/selector guard, device resolve, ref-frame expiry, record) instead of reusing it, because the shared function had no seam for keyboard's bind-and-execute admission — only the legacy requireCommandSupported + dispatchCommand path. That left the shared orchestrator with one caller instead of two, and set a precedent that would fork a new copy for each of the 28 remaining session-route migrations. Gave runSessionOrSelectorDispatch an `execute` parameter: the orchestration (guard, resolve device, admit-then-execute, expire ref frame if mutating, derive and record next session) stays in one place, and callers supply their own admission/execution strategy. Extracted `legacySessionDispatchExecute` for the still-legacy capability-gate-then-dispatchCommand shape `handleTriggerAppEventCommand` (the remaining legacy caller) now passes explicitly, and `keyboardSessionExecute` for keyboard's bind-and-execute shape. Deleted the now-fully-redundant `executeBoundKeyboardCommand` — its result recording duplicated what the shared orchestrator's tail already does. pnpm typecheck, check:fallow, check:layering, the full daemon test suite (321 files / 2271 tests), and pnpm check:affected --run are green. * refactor: extract limrun facts-runtime.ts; discriminate KeyboardDismissResult by owner app-log-runtime.ts was still 589 lines after the shared-abstraction fixes; moves fact assembly (limrunAppLogFacts/limrunAppLogRecoveryFacts/limrunLifecycleFacts/deploymentOptions) to a new facts-runtime.ts and the shared device-identity predicate to device.ts, the leaf both files already depend on. app-log-runtime.ts is now 336 lines. KeyboardDismissResult was an 11-field optional bag with executeKeyboardDismiss separately re-deriving platform from the device and projecting subsets by hand. Each owner (android, apple, harmonyos) now tags its own result with a `kind` discriminant, so an owner can only ever produce its own shape, and the daemon derives the wire `platform` label from `kind` instead of guessing from the device a second time. Wire output is unchanged. * fix: expire ref frame before the mutating call, not after; extract session/selector dispatch; derive catalog operations from facts runSessionOrSelectorDispatch awaited execute(device, session) — which bundled admission and the mutating invocation together — before expiring the ref frame, so a rejecting or timed-out invocation left a stale frame active (ADR 0014 requires expiry immediately before the mutating call, with no success-only rollback). Split the execute thunk into `prepare` (admission only) + a deferred `execute` invocation, so the orchestrator can expire between them regardless of how the invocation resolves. Added a regression test proving the frame still expires when the invocation rejects. Extracted runSessionOrSelectorDispatch and its keyboard/trigger-app-event callers into a new session-selector-dispatch.ts, matching this file's own convention of one file per command-group (session.ts shrinks from 571 to well under its 500-line budget). bindAdmittedLocalInteractorOperations/bindAdmittedProviderInteractorOperations accepted both a facts object and a separately hand-maintained `operations` array naming the same keys — a second source of truth that could drift from what the facts actually admit. Removed the array; the binder now walks the fixed set of navigation operations and lets each owner's own facts decide what binds, exactly as before but with one source of truth. * style: reformat legacySessionDispatchExecute call in session-selector-dispatch.ts * refactor: derive catalog operation list from one canonical tuple; move keyboard orchestration tests NAVIGATION_INTERACTOR_OPERATIONS was declared as a plain readonly array independently of the NavigationInteractorOperation union it walked, so a future union member could compile without ever being added to the walk list, silently preventing an admitted fact from binding. Made the tuple the single canonical value: the union type is now derived from it via `(typeof TUPLE)[number]`, so LOCAL_BINDERS/PROVIDER_BINDERS' Record<NavigationInteractorOperation, ...> completeness is checked against the same tuple, not a separately hand-kept list. Added a regression test binding all seven operations at once to pin the runtime walk, independent of the type-level guarantee. Moved the four keyboard-orchestration tests (the two ADR 0014 ref-frame seam tests plus the two session/selector-guard tests) out of the mixed appstate/perf test file into a new session-selector-dispatch.test.ts, colocated with the file they exercise. Strengthened the rejection regression test to assert the frame is already expired from inside the rejecting keyboardDismiss callback itself, pinning the exact pre-invocation seam rather than only checking the end state after the dispatch settles. * fix: restore back/home/orientation/tv-remote/keyboard-runtime exports lost in rebase Rebasing onto origin/main dropped these five package.json export entries during conflict resolution (the granular-subpath commit's package.json changes silently lost during merge). Restored, confirmed by pnpm typecheck across all 17 workspace packages and the full unit-core suite (1023 files / 7581 tests). * test: pin the exact point the live iOS email field value goes missing Two prior CI runs on this PR saw the seeded email field ("ada@example") end up containing only a typed suffix (".test") by the time the flow reads it back at the end — after fill, keyboard dismiss, coordinate refocus, and type. Since this PR touches executeKeyboardDismiss's response shaping, the reviewer asked to disprove keyboard dismiss as the cause rather than assume the pre-existing dropped-keystroke flake pattern applies. Added two read-back checkpoints: right after seeding (before dismiss runs at all) and right after dismiss (before the coordinate refocus + type steps that follow). If both hold "ada@example", the loss happens during refocus/type, not dismiss — matching the documented flake, not a regression in this PR's diff. * refactor: make keyboard status/enter owner-discriminated too; trim review-round prose KeyboardStatusResult and KeyboardEnterResult were bare objects; executeKeyboardStatus and executeKeyboardEnter derived the wire platform label from device.platform via keyboardPlatformLabel, the same re-derivation already fixed for dismiss. Each owner now tags its own result with a kind (android's status/enter as 'ime-probe' and 'android-acknowledged', harmonyos's enter as 'harmonyos-acknowledged', apple's enter as 'visibility-echo'), and the daemon derives platform from a kind-keyed lookup table for all three actions. keyboardPlatformLabel and its isIosFamily import are gone — nothing derives platform from the device anymore. Android and HarmonyOS's enter acknowledgments are structurally identical (empty besides kind), so the discriminant alone — not result shape — is what tells the daemon which owner actually ran. Added a harmonyos enter test alongside the existing ios/android ones so all three owners are covered for both dismiss and enter's kind-to-platform mapping. Also trimmed several comments that narrated which PR review round motivated them down to just the durable invariant or rationale — the type shape, test names, and assertions already carry the proof. |
||
|
Michał Pierzchała
|
c4b1a6131e |
dx(test): opt-in worker-count override for solo local vitest runs (#1964)
* dx(test): opt-in worker-count override for solo local vitest runs resolveVitestMaxWorkers() caps local runs at 2 workers so parallel worktrees and spawn-heavy tests keep headroom, but a solo run that owns the machine pays 6x on a 12-core host for no benefit. Add AGENT_DEVICE_VITEST_MAX_WORKERS to opt in to a higher cap. It is clamped to os.cpus().length so a runaway value can't oversubscribe the host, and it is a no-op in CI (CI already derives its own worker count). A missing, blank, non-numeric, non-integer, or non-positive value falls through to the existing default cap rather than throwing. Default (unset) behavior is unchanged. Closes #1962 * docs: tighten the worker-override note to fit the agent-guidance budget docs/agents/testing.md sits at a 10,000-byte per-file ceiling enforced by check:agent-guidance, and the first phrasing pushed it to 10,065. Restate the override in one tighter bullet that leads with the "solo run only" caveat, which is the constraint a reader most needs. * fix(test): clamp the worker override with os.availableParallelism() Node documents cpus().length as unfit for sizing application parallelism: it ignores CPU affinity and cgroup limits, so it can report a pool wider than the process may actually use. Clamping against it would inflate the very ceiling this override's safety clamp exists to enforce. availableParallelism() honors those constraints, so the clamp now means what it claims on constrained hosts. Test updated to match. * test: keep the resolver cases in the already-included setup test Review feedback: a new test file beside the resolver, plus its entry in vitest.config.ts's unit-core include list, is a change to test discovery that the mutation lane's `vitest related` graph reads. Fold the override cases into src/__tests__/hermetic-env-setup.test.ts, which is already in the unit suite and already imports the resolver, and drop the config edit entirely so this PR no longer touches test discovery at all. Same six assertions, no coverage lost. |
||
|
Michał Pierzchała
|
7aaa559e29 | perf(android): bound occlusion coverage memory (#1982) | ||
|
Michał Pierzchała
|
beaa1e689a |
refactor(recording): move gesture telemetry out of the daemon zone (#1985)
`recording-telemetry.ts` imports only `node:fs`, `node:path` and `@agent-device/contracts/platform` — it has no daemon dependency and is recording-domain file serialization, not daemon behavior. Sitting under `src/daemon/` made the platform-runtime composition root reach up into daemon-server (rank 4) for it. Move it to `src/recording/telemetry.ts`, alongside `output-path.ts` and `overlay.ts`, which share its shape (node builtins plus contracts/kernel). Pure rename: the module body is unchanged and the three import sites are retargeted. This removes one of the composition root's four upward value edges — the only one reaching rank 4. Confirmed with `pnpm depgraph`: those edges drop from 4 to 3 and the daemon-server target is gone, with cycle, back-edge and type-inversion counts unchanged. Claude-Session: https://claude.ai/code/session_01WWLBCBDBpdR8z1aCmDerXS Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
c78095cbc7 |
fix(macos): sign nested runner code so the host runner app can launch (#1979)
* fix(macos): sign nested runner code so the host runner app can launch
macOS runner builds pass CODE_SIGNING_ALLOWED=NO, so Xcode embeds
Testing/XCTest/XCUIAutomation.framework without re-signing them while the
embed step drops their sealed Modules/*.swiftinterface entries. Those three
nested seals are therefore always broken in a fresh macOS build.
repairMacOsRunnerProductsIfNeeded signed only the outer bundle, which leaves
the nested seals untouched. `codesign --verify --deep --strict` kept failing,
macOS refused to launch the app ("AgentDeviceRunnerUITests-Runner.app is
damaged and can't be opened"), and the runner was killed before it could
establish a connection. codesign still exited 0, so the repair reported
success on every attempt while fixing nothing.
Sign with --deep so nested code is re-sealed, and re-verify afterwards so a
repair that repairs nothing raises RUNNER_PRODUCT_REPAIR_FAILED instead of
passing silently. The runner is ad-hoc signed with no entitlements, so --deep
discards nothing.
* test: use the shared mkdtempForTest helper for the runner repair fixtures
The suite redirects TMPDIR per run and removes it after every worker, so the
hand-rolled afterEach cleanup was redundant. Use mkdtempForTestSync like the
sibling runner tests do.
* fix(macos): sign runner products bottom-up
* fix(macos): discover embedded runner code
|
||
|
Michał Pierzchała
|
d1fc80fafb | fix(android): update snapshot quality lifecycle import (#1980) | ||
|
Michał Pierzchała
|
3e6ff4cd06 |
fix(web): scope disabled detection to the ref-bearing annotation group (#1951)
* fix(web): read enabled=false from the snapshot [disabled] annotation The agent-browser refs payload carries only role+name, so web snapshot nodes never reported enabled=false; the [disabled] bracket annotation in the snapshot text (emitted for both the attribute and aria-disabled) went unparsed. Quoted label text is stripped before scanning so a literal "[disabled]" inside a label does not flip the flag; explicit refs metadata still wins. Focus state is not emitted by the pinned backend and remains metadata-only. * refactor(web): share snapshot annotation parsing |
||
|
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
|
e0cd06e567 |
test(snapshot): add cross-runtime presentation conformance (#1973)
* test(snapshot): add cross-runtime presentation conformance * fix(ios): preserve acquired snapshot actionability * chore(ios): retain existing XCTest selection name * test(snapshot): cover nested cumulative clipping |
||
|
Michał Pierzchała
|
6eb74d08ce |
feat(android): bound snapshot presentation quality (#1972)
* feat(android): bound snapshot presentation quality * fix(android): bound presentation footprint work * fix(android): account for presentation scan work * fix(android): budget scoped snapshot presentation * fix(android): admit bounded snapshot presentation * fix(android): validate presentation per window |