mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
scratch/depgraph-report
259 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michał Pierzchała
|
56b72c5cf7 |
refactor(boundaries): put shared contracts below their consumers, gate the result (#1405)
* refactor(boundaries): move shared contracts below their consumers Acts on the depgraph findings: type-only edges are invisible to R5, so vocabulary that everything depends on had drifted above the zones that use it. - contracts/: the four platform-plugin facet tags (LogBackend, RecordingBackendTag, PerfMetricsSamplerTag, PlatformGatedProviderResolverKey) now live beside the plugin contract itself, which also moves out of core/; NetworkEntry moves next to the command surface that renders it; and the click-button, recording-export-quality, interactor-types and runner-lease-context vocabularies move down out of core/. - (root) drops from 29 files to 13: the internal *-contract/output/annotation modules move into contracts/, kernel/ (daemon-error, observability-redaction beside kernel/redaction), core/ (batch-policy, an ADR 0008 projection), commands/ (cli-command-aliases) and remote/ (upload-progress, upload-stream). What remains is entrypoints and the composition roots that R2 requires to sit outside the spine. - utils/ joins the ranked spine at rank 1 after its only two upward files move to the zones they were reaching for (cli/resolve-cli-options, cli-schema/cli-config), putting ~336 value edges under the gate. - Internal imports that routed types through the client-types re-export hub now name their real source. Type-only spine inversions drop from 61 to 35; the remainder is two clusters (client/client-types.ts and the ADR 0003 daemon facet). No behaviour change: 4470 unit tests and the layering gate pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * style: merge the duplicate contract imports the tag moves created Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * refactor(imports): name the declaring module, share find's argument rules Two follow-ups from re-measuring the graph after the boundary moves. 1. 89 type imports across 79 files routed through a re-export hub in another zone: `CliFlags` reached through commands/cli-grammar/flag-types.ts (52) when it is declared in contracts/cli-flags.ts, the replay suite result types reached through daemon/types.ts when they are declared in contracts/replay.ts, the doctor types through a daemon handler module, and so on. Each hop invented a cross-zone edge the architecture never asked for — including every apparent replay -> daemon and utils -> commands dependency. They now name the module that declares them. Within-zone hops are left alone; those are a local style choice, not a boundary claim. 2. `find`'s three positional/flag checks existed in both daemon entry points with hand-repeated messages, and the copy in dispatchFindReadOnlyViaRuntime was unreachable — its only caller validates first. Both now call checkFindArgs in selectors/find.ts, beside parseFindArgs and isReadOnlyFindAction, for the reason that module's own comment already gives: so the two paths cannot disagree. The refusal is returned rather than thrown, because the two mechanisms are not observationally identical in the session event log. Type-only spine inversions: 61 -> 35. 4470 unit tests and every gate pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * feat(layering): ratchet type-only spine inversions (R6) R5 ignores type-only edges by design — they cost nothing at runtime and do not affect cold start — so nothing was watching the direction they point. Ranking them the same way found 61 inversions, including contracts/ and utils/ declared in terms of rank-4 zones. 26 are fixed by the preceding commits; R6 pins the rest per zone pair so they can only shrink, and a new pair fails outright rather than being added to the baseline. The two remaining clusters each need their own change, and the baseline says so: the per-command Options/Result vocabulary declared inside the public Node-client surface, and the ADR 0003 daemon facet shape that core's descriptor registry composes. Both ratchet directions are covered: growth fails, and shrinking without lowering the number fails too, so the baseline cannot quietly stop describing the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs: record the import-graph findings behind this refactor A dated snapshot, not a normative document: when it disagrees with scripts/layering/, the gate wins. The graph tool that produced it lives on the claude/depgraph-viewer branch, deliberately out of this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * refactor(selectors): state the shared selector argument rules once R2 (commands-floor) forbids the daemon from importing commands/, and that is the right call: commands/ is the client-side surface — its only consumers are cli/, cli-schema/, mcp/, client/ and the composition roots — while the daemon is the executor on the other side of the wire. ADR 0008 protects exactly that seam. Relaxing R2 would let the executor depend on a client projection and pull CLI grammar and output formatting into the daemon's bundle. But the rule does force duplication: the daemon must validate independently because it accepts requests from any client, so 10 refusal messages existed in both zones. The only place a shared rule can live is below both, and selectors/ already held the parsers (splitIsSelectorArgs, splitSelectorFromArgs, isSupportedPredicate) and even the `is` predicate message — just not the checks that use them. Three drifts had already appeared in the `is` predicate rule alone: - commands/interaction/selectors.ts re-implemented the predicate list as an inlined seven-way `!==` chain while importing the message and hint from selectors/predicates.ts, so adding a predicate to the shared list would not have reached the CLI grammar. - That inlined chain compared the raw token, so the CLI rejected `is TEXT ...` while the daemon it hands the command to accepts it. The CLI now matches the executor; this is an intentional alignment, not an accident. - isCommand raised the same refusal without IS_PREDICATE_USAGE_HINT, so whether an agent got recovery guidance depended on which layer noticed first — the failure mode ADR 0010's audit calls out. checkIsPredicate, checkIsArgs, checkGetFormat, checkElementTargetArgs and checkWaitText now hold those rules, each beside the parser it wraps, and report a refusal rather than choosing how to raise it: the daemon returns a response, the command surface throws. Those mechanisms are not interchangeable — they write different session events — so the shared check stays out of that decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * feat(daemon): give ADR 0014's ref frame one transition, pin SessionState owners `SessionStore.get()` returns the live record out of a private Map and `set()` re-puts the same reference, so every `session.<field> = …` in the daemon is a durable write to store-owned state: 57 of them across 17 files, against 26 `set()` calls that are therefore ceremonial. Nothing at the store boundary can check what those writes are supposed to keep true. Measuring which module writes which field showed the problem is narrower than the raw count suggests — 16 of 27 fields already have exactly one writer. The sharp case is ADR 0014's ref frame: `refFrameState`, `refFrameScope`, `refFrameTree` and `refFrameGeneration` must move together or the frame is incoherent (an `active` state with a stale tree resolves refs against a namespace nobody authorized), yet complete issuance wrote them in ref-frame.ts and partial issuance wrote the same four in session-snapshot.ts. ref-frame.ts's own header claims to be "the single owner of the frame's transitions", and session-snapshot.ts documented itself as the exception. Both forms now go through `activateRefFrame`; they differ only in scope. `recordSession` deliberately moves alone in two paths (recording without arming a publication), so the save-script cluster gets no invented abstraction — it gets ownership instead. R7 records every field's owner and stops the set from growing quietly: a new SessionState field must declare one, a foreign write fails naming the owner to call, and an owner that stops writing must be removed so the table cannot drift into fiction. Field names are read out of the `SessionState` declaration, so a daemon module with an unrelated local named `session` — a provider or runner session — cannot trip it. 4475 unit tests and every gate pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs: record the reference semantics and refresh the findings SessionStore.get/set now document that the record is handed out live, since that is the fact behind R7. The findings snapshot picks up the resolved R2 question, the ref-frame consolidation and the two new gate scopes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * refactor(boundaries): rank every satellite zone, extract the provider port Second-order effect of the earlier rounds. With `utils` on the spine and `(root)` emptied of shared contracts, the eleven zones that were unranked "because ranking them would invent an order the architecture had not committed to" turned out to have a consistent rank already — the order was there, unasserted. Solving the constraint system showed one blocker: `utils/remote-config.ts` projected a remote-config profile into `CliFlags` while reaching up into `remote/`, and its only three consumers were in `cli/`. It moves there as `cli/remote-config-flags.ts`, and every satellite zone joins the spine. Ranked coverage goes from 730/895 files to 882/895. Only `(root)` stays out, and now for one stated reason: R2 forbids `daemon/` from importing `commands/`, so the files that wire them compose the spine from above. Ranking them exposed 22 type-only inversions R6 had never been able to see, and they were concentrated rather than scattered: - The device-provider port. `providers/` and `cloud-webdriver/` implement what the daemon calls, so both sides name `DeviceLease`, `LeaseLifecycleProvider`, `LeaseLifecycleContext` and `DeviceInventoryProvider` — now declared in contracts/device-provider.ts, below both. The adapters also imported the daemon's NARROWED `DaemonRequest` while only ever reading `req.flags`; they now name the public one from kernel/contracts. - `MetroPrepareKind` and the remote-config profile field groups move to contracts/ for the same reason: the command surface validates them and contracts/cli-flags.ts is composed from them. Two clusters remain, ratcheted with their reasons in TYPE_INVERSION_BASELINE: the client-types vocabulary, and `SessionAction`, which needs `CommandFlags` and `DaemonBatchStep` to move with it. Also fixes two things CI caught: the eight type re-exports my earlier import redirection orphaned (none published through any src/sdk/* entrypoint, so no public surface changes) and `isSupportedPredicate`, now module-private since `checkIsPredicate` is the admission API. `fallow-baselines/health.json` is keyed by path, so the moved cli-config entry moves with the file rather than being regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * fix(selectors): use the admitted predicate, not the raw option Review finding. `isCommand` called `checkIsPredicate` and then kept reading `options.predicate` for the capture policy, the `exists` branch, `evaluateIsPredicate`, the failure message and the returned result. Admission normalizes case, so an upper-case predicate was let past the gate and then evaluated against lower-case branches: `EXISTS` skipped its own branch and fell through to the generic path, and the result echoed the raw token. I widened admission at that surface without threading the normalized value through it — the CLI-grammar surface in the same change does use the admitted value. Every decision after admission now reads it. Two tests, both verified to fail without the fix: - a production-route regression driving `device.selectors.is` with `EXISTS`/`TEXT`, plus one pinning that an unknown predicate is still refused WITH the ADR 0010 usage hint; - a surface parity gate (selectors/__tests__/is-argument-surface-parity.test.ts) in the repo's existing parity style, asserting the daemon and CLI-grammar surfaces reach the same verdict and hand the same normalized predicate downstream across an input table. A helper-only test cannot catch a surface that admits correctly and then discards the result, which is what happened here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs: name the pre-push gate, and the formatter's path allowlist Both misses in this PR's review were process, not judgement, and the docs pointed the wrong way for both. AGENTS.md said "prefer the aggregate package.json scripts" without naming which aggregate, and CONTRIBUTING listed `pnpm test` and the targeted checks but never `pnpm check`. `check:tooling` looks like the gate and is a subset of it: it stops before the Fallow audit, so the dead exports this PR introduced passed a clean `check:tooling` and failed CI. Both files now name `pnpm check`, say what it covers, and say what it cannot (the device matrix). The same gap produced a second mistake twice: `oxfmt <path>` reformats whatever you point it at, while the repo's `format` script is an allowlist that excludes `scripts/` and every `.md`. One run reformatted 50 unrelated script files into a commit; the next nearly did it to AGENTS.md. AGENTS.md now says to run `pnpm format`, never `oxfmt <path>`. It also records the rule that cost a CI cycle: Fallow's baselines are keyed by path, so a renamed file needs its baseline entry moved, not the baselines regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * revert: undo stray formatter output across docs and scripts Three separate `oxfmt <path>` runs in this branch reformatted files the repo's `format` script deliberately excludes: 55 files under scripts/maestro-conformance plus scripts/perf, sync-mcp-metadata and the slow-test reporter, and 12 markdown files including six ADRs and docs/agents/. All of it was whitespace, quote style and markdown table padding — no content — but it inflated the diff a reviewer has to read and would have rewritten prose ownership across files this change has no business touching. All 70 are back to their origin/main content, so the diff outside src/ is now exactly this change's scope: three docs, scripts/layering, the Fallow baseline, and five provider integration tests. The rule this violated is now in AGENTS.md: run `pnpm format`, never `oxfmt <path>`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * style: reformat two provider tests with the repo's pinned oxfmt `pnpm format:check` failed in CI on the two files whose imports I merged by hand. The repo pins oxfmt 0.42.0 as a devDependency and both `format` scripts invoke `./node_modules/oxfmt/bin/oxfmt`; I had reformatted with `npx oxfmt`, which resolved 0.60.0, and the two versions disagree about wrapping a 100-column import. This is the rule AGENTS.md already states — run `pnpm format`, never oxfmt directly — so there is nothing to add to the docs, only to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * fix(ci): install deps for the layering guard, and gate the zero-dep contract The Layering Guard job failed with ERR_MODULE_NOT_FOUND on `oxc-parser`. The job ran with `install-deps: false` — no `pnpm install`, so no `node_modules` — and R7 had started parsing the daemon with oxc-parser instead of matching assignment operators with a regex. `pnpm check:layering` passed on every local run, because locally `node_modules` is always there. The job now installs dependencies. The alternative was to put R7 back on a regex, which cannot see `??=` or a computed `session[key] =` write, so it would trade a correct rule for a fast job. That leaves the interesting part: the zero-dep contract is real for the jobs that keep it, and it is invisible to every local run, which is the worst combination a constraint can have. R8 makes it checkable. It reads the zero-dep job list out of `.github/workflows/` rather than restating it — declaring a job zero-dep is what puts it under the rule — walks each job's entry scripts and their whole relative-import closure, and requires every specifier to be a Node builtin or another repo file. A zero-dep job whose entry scripts the scan cannot identify fails too, so the rule cannot be escaped by changing how the job invokes them. Specifiers come from oxc-parser's module record, not a line scan. The closures include `--test` files, and a test about imports naturally embeds import syntax in a fixture string; the line scanner reported two such phantom violations in model.test.ts before the switch, which is how a gate stops being trusted. Verified by re-running the real gate against three injected regressions: the layering job back on `install-deps: false` (reproduces the exact CI failure, pointing at session-state.ts:24), a package import added to the still-zero-dep affected-selector closure, and a zero-dep job whose run step names no script. Also corrects the CONTEXT.md spine paragraph, which still described the satellite zones as deliberately unranked after they had all joined the ranked spine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * fix(layering): make R7 exhaustive, and follow session records through aliases Review finding: `SESSION_STATE_FIELD_OWNERS` covered 27 of `SessionState`'s 42 fields and nothing asserted parity, so a new field could be added and pass the gate by being invisible to it. R7's advertised claim — "every SessionState write is inside its declared owner" — was broader than what it checked. Investigating that turned up a second, larger gap the finding did not name: the scan only recognized a binding literally named `session`. The daemon names these records by role, so `nextSession`, `provisionalSession`, `completedSession`, `preRunSession` and `preEntrySession` were all invisible — and three of those writes were genuine violations R7 existed to catch: src/daemon/snapshot-runtime.ts:256 nextSession.snapshotScopeSource src/daemon/snapshot-runtime.ts:265 nextSession.snapshotGeneration src/daemon/handlers/session-replay-runtime.ts:707 preEntrySession.pendingRecordAndHeal The first two are the #1076 versioned-ref invariant: the generation advances exactly when the stored tree is replaced. That rule lived in `setSessionSnapshot` and had acquired a second statement of itself in snapshot-runtime.ts, whose own comment admitted the bypass. It now goes through `setSnapshotLineage` in the owning module. The third clears a watermark stamped by session-replay-resume.ts; `clearPendingRecordAndHealWatermark` puts the clear beside the stamp. Gate changes: - Binding detection accepts aliases, paired with the existing declared-field filter so an unrelated `…Session` local only registers if it also writes a field SessionState owns — where the remedy is the same anyway. - `fieldClassificationDrift` asserts parity in all three directions: unclassified, in-both, and naming a field SessionState no longer declares. - `STORE_OWNED_SESSION_STATE_FIELDS` classifies the 11 fields the store establishes at construction. It is a positive claim, so a direct write to one fails and names both remedies. - Four fields the widened scan made visible (`lease`, `deviceClaim`, `appName`, `saveScriptComplete`) got real owners. `nextSnapshotGeneration` is now module-private: replacing its only external call site orphaned the export, which `pnpm check` caught via Fallow. Verified against three injected regressions: a new SessionState field with no direct write (the reviewer's exact scenario), a foreign write through an alias binding, and a direct write to a store-established field. All three rejected. `pnpm check` green, 4486 unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs(daemon): correct the snapshot-lineage claim, and pin the real contract Device verification of the snapshot-lineage route found that a ref pinned before a `diff` keeps resolving with no pinned-ref warning. That is the designed ADR 0014 behaviour, not a regression — the comment describing it was wrong, and I propagated it. `main`'s comment in snapshot-runtime.ts said a diff "leaves client refs pinned to the previous generation, which is exactly what the pinned warning diagnoses". The counter and the authorization epoch are different clocks: - `diff` passes `issuesRefsToClient: false`, so it never reactivates the frame; - `resolveRefStalenessWarning` compares a pin against the frame EPOCH, not the observation counter, and its own comment says why — a capture that bumped the counter must not make a valid pin from the issuing frame look stale. So advancing the counter is not the same as invalidating client refs, and the observable the comment promised does not exist. I carried the sentence into `setSnapshotLineage`'s doc when the transition moved, and then into a hardware verification request, which cost a reviewer a device run against a false claim. `setSnapshotLineage` itself is unchanged and was a pure move: same expressions, same inputs as the inline assignments it replaced, so this route behaves exactly as it does on main. A comment that contradicts the code should be an assertion instead, so the contract is now pinned in session-snapshot.test.ts: the diff advances the counter, preserves the epoch, leaves the pre-diff pin resolving without a warning, and still warns for a pin from a different frame. Verified to fail when the epoch comparison is swapped for the counter. A second test covers the keep-current branch, which had no coverage. `pnpm check` green, 4488 unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
287cc18c29 |
fix(gestures): fail pre-removal gesture forms at .ad parse time (#1216) (#1393)
* fix(gestures): fail pre-removal gesture forms at .ad parse time (#1216) #1315 removed the timed forms of `swipe`, `gesture fling`, and `gesture swipe` and `gesture rotate`'s `velocity`, but shipped without the migration guide, the repository sweep, or the parse-time error that issue #1216's own checklist gates a removal on. The sweep finds what that left behind: both `06-swipe-gestures.ad` integration fixtures still carried the 5-argument swipe and would fail at replay, two tests still asserted the removed shapes, and two branches still read the retired positional. Argument arity for every public gesture syntax now lives in one table keyed off the canonical `GESTURE_KINDS`, so a new kind cannot skip it and a form removed from the CLI is removed from `.ad` in the same edit. Both callers read it: the CLI argv parse, and a new `.ad` preflight. A stale script now fails when it is parsed — before the replay executes any device action — naming the line and computing its rewrite, instead of running up to that step and failing as a repairable divergence. The preflight checks arity only: `${VAR}` tokens resolve after planning, and interpolation never splits a token, so the count is decidable while the values are not. Deleting the dead duration read in `readSwipeGeometry` would have left `replay export` emitting no duration, handing Maestro's 400ms default to a gesture the script runs at 100ms, so the export now states `duration: 100`. `.ad` positional gesture parsing is NOT removed. Its only remaining callers are the CLI argv parse and the `.ad` line parse, both the current public syntax rather than a bridge to an older one, so there is nothing to migrate off. ADR 0013 records that and drops the "compatibility" framing that made it read as debt. Both migrated fixtures verified on real devices with the repo's own CLI: iOS simulator 34.9s, Android emulator 45.9s. * fix(gestures): reject removed swipe input at the Node/MCP boundary Review findings on d88c6ed8. P1: `interactionDaemonWriters.swipe` hand-projects five fields, so a JavaScript caller's `durationMs` was dropped before the daemon's `readSwipeInput` could reject it and a default-duration fling ran instead — the exact silent reinterpretation the guide promises does not happen. `gesture` was already safe because its writer runs `readGestureInput` -> `readGesturePayload`, which rejects the removed keys; `swipe` was the one surface with no reader of its own. The rejection now lives in contracts and is shared by the client writer and the daemon handler, so there is one rule and one message. The SDK regression covers all four removed keys and asserts the transport is never reached; reverting the writer call fails it on `swipe durationMs`. P2: the preflight's retired-slot test required a numeric token, so `swipe 197 650 197 300 ${DURATION}` fell back to bare usage text. An unresolved `${VAR}` now counts as the retired slot and is carried into the pan rewrite, while a stray flag or word stays a plain usage error. P2: the removal shipped in 0.20.0, not 0.21 — removal commit |
||
|
Michał Pierzchała
|
75b5bc5d6d |
feat: add first-class Vega VVD TV support (#1396)
* feat: add first-class Vega OS TV support * fix: scope Vega support to VVD * fix: tighten Vega platform boundaries |
||
|
Michał Pierzchała
|
877e68fe30 |
fix(cli): compact stale device status (#1388)
* fix(cli): compact stale device status * fix(cli): quote stale status selectors |
||
|
Michał Pierzchała
|
d9b583b950 |
test: serialize client-metro and harden the unit suite against ambient daemon env (#1365)
* test: serialize client-metro subprocess-stub test to end contention flakes src/__tests__/client-metro.test.ts stubs npx and the package managers on PATH and spawns a real Metro dev server per case, so each case waits real subprocess time (~570-910ms measured). Run in the unit-core project at ~7x file parallelism it contends for CPU with every other stub-spawning file; a starved spawn pushes production down a generic failure path that returns a different error than the assertion expects. This is the same contention flake #1362 serialized runtime-hints.test.ts and apple/core/index.test.ts for, but this file was not included in that batch (observed failing a full test:unit run while passing 20/20 in isolation and green on a re-run). Add it to SUBPROCESS_STUB_TESTS so it joins the fileParallelism:false, maxWorkers:1 subprocess-stub project, so at most one real-stub-spawning file spawns at a time. Injecting the spawn budget is not the lever here: the fake dev server becomes ready fast (no waited-out timeout to inject), the per-case cost is a genuine subprocess spawn, and each case is already under the 2.5s slow-test budget so the gate never flags it. Removing the cost would mean mocking the very spawn/args/package-manager-detection path the file exists to verify, and an exec-options DI seam is forbidden by the CI gate (AGENTS.md). Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * test: make the vitest suite hermetic against ambient daemon env A machine actually running agent-device — including this repo's own remote dev containers — exports AGENT_DEVICE_DAEMON_BASE_URL and AGENT_DEVICE_DAEMON_AUTH_TOKEN pointing at a live daemon. Production flag-default resolution folds those into every command's input and connection config (resolveConfigBackedFlagDefaults -> readEnvFlagDefaults, plus the daemon client's own env fallbacks in daemon-client-lifecycle), so a configured host silently diverges from CI, which runs with them unset: - command-tools and cloud-connect-profile assert exact command input / profile shapes and gain phantom daemonBaseUrl/daemonAuthToken keys. - daemon-client and daemon-client-lifecycle take the remote-daemon path ("Remote daemon is unavailable") instead of the local one they exercise. 28 tests across those four files fail deterministically on such a host — fast, in isolation, not contention — while passing on CI. Add a shared setup file that deletes the two ambient daemon-connection vars before each test file, wired into every vitest project alongside the existing process-memo reset, so a configured host matches CI. Tests that need these set assign their own value or pass an explicit env object, which runs after this setup module loads and is therefore unaffected. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * test: add a regression guard for the hermetic-env setup CI runs with AGENT_DEVICE_DAEMON_* unset, so it can never exercise the scrub that hermetic-env-setup.ts performs — deleting the setup file or forgetting to wire a project would leave the whole suite green. Add a guard that closes that gap two ways: - A static check that every configured vitest project lists the setup file in its setupFiles (catches an unwired project). - A real vitest child, launched with both daemon vars set, running a probe fixture that proves a wired project sees them scrubbed — plus a negative control (same vars, setup disabled) proving the probe actually detects the leak, so a green result is the setup working, not a no-op. A control var the setup must not touch proves the child inherited the injected environment. The two children run concurrently to stay within the unit wall-clock budget, and the file joins the serialized subprocess-stub project since it spawns real vitest processes. Ignore the fixture config's default export in fallow — vitest loads it by path rather than importing it, the same class as the other tool-config default exports already listed there. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * fix(test): route the hermetic-env guard's vitest child through runCmd AGENTS.md hard rule: TypeScript process execution must go through src/utils/exec.ts, never raw spawn/spawnSync. The guard's runProbe spawned the vitest child with node:child_process spawn directly, bypassing the shared timeout, process-tree cleanup, normalized failure, and diagnostics behavior. Replace it with runCmd invoking the repo-local vitest CLI (node node_modules/vitest/vitest.mjs) with allowFailure: true and a bounded timeoutMs, asserting the returned exitCode. allowFailure surfaces the negative control's non-zero exit as data instead of throwing, so that assertion keeps working; the two probes still run concurrently to stay under the unit budget. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * fix(test): make the hermetic-env guard's probe cleanup timely and complete Follow-up to review. Two process-tree cleanup gaps in the guard's runProbe: - It gave the nested vitest a 60s timeout while the enclosing test used vitest's default (5s) timeout, so a hung probe would time the parent test out first and leave the child running (orphaned). - runCmd only process-group-kills when detached:true; without it a timeout kills only the direct child, so vitest worker descendants could survive. Run the probe detached so a timeout kills the whole process group (the vitest child and its workers), and set the child timeout (20s) strictly below an explicit enclosing test timeout (40s) so a hang is reaped by runCmd before vitest abandons the parent test. The repo-local node/vitest invocation, allowFailure, exit-code assertions, and the concurrent positive/negative probes are unchanged. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK * test: replace the child-process hermetic-env guard with an in-process test The previous guard spawned a real vitest child to exercise the setup through vitest's setupFiles machinery, which dragged in process-group cleanup, timeout ordering, a fixture config + probe, and a fallow suppression — a pile of scaffolding around a scrub that is two delete statements. Prove the same contract in-process instead: - Behavior: set the daemon vars, vi.resetModules(), re-import hermetic-env-setup, and assert they are gone. This exercises the real import-time scrub on any host (CI included, where the vars are otherwise absent) and fails if it regresses. - Wiring: assert every configured vitest project lists the setup in setupFiles. Runs in ~5ms in unit-core with no subprocess, so it also drops out of SUBPROCESS_STUB_TESTS and removes the fixtures and the fallow ignore entry. Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGCchHyrPhTqtXniWuERYK --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
d237bc555d |
chore: remove verified dead code and migration scaffolding (~700 LOC) (#1367)
* chore: remove verified dead code and migration scaffolding Multi-agent audit of accumulated waste, every finding adversarially verified against call sites, git history, and the published surface before removal. Net -710 lines. - delete src/core/platform-descriptor/ (superseded ADR-0009 migration scaffold; parity tests now assert an inline table) - remove test-only seams: registry introspection exports, CommandFacet.extraDaemonWriters, MaestroEngineOptions.timing - remove dead flexibility: backend capability allow-list, screenshot-diff maxRegions, CloudWebDriverSupportLevel 'partial', clearFirst on the TS+Swift runner wire contract - remove dead deprecated surface: --session-locked / --session-lock-conflicts aliases (hard migration error now points at --session-lock), replay export --format single-value enum, unused Lease*Payload contract types, runtime-layer rotate duplicate - collapse pass-throughs/duplication: withRetry adapter, default-cloud-artifact-provider, connect-profile client-id hashing (3x sha256 impls -> one helper, byte-identical output), shared scripts walker, cloneValue -> structuredClone, fill-diagnostics moved into android/ BREAKING CHANGE: --session-locked and --session-lock-conflicts now fail with a migration error pointing at --session-lock; replay export --format is removed (Maestro was the only value); Lease*Payload types are dropped from the ./contracts subpath. * chore: satisfy fallow gates tightened by #1363/#1364 after rebase - drop the consumer-less AndroidFillVerificationNode re-export - reuse requireSnapshotSession in resolveSnapshotForRef instead of inlining the same authorized-frame resolution (fallow clone group); the helper's return type now guarantees the session it already throws for * chore: address review — keep cloud-webdriver partial capability metadata The partial/supported/unsupported levels and their notes are part of the lease-response capability contract for genuinely limited operations (Appium page-source snapshots, upload-then-install), not dead scaffolding. Restore them and the asserting tests unchanged from main. Also add the missing CHANGELOG entry for the Lease*Payload type removal from agent-device/contracts. |
||
|
Michał Pierzchała
|
c0fc822e80 |
chore: remove dead code and tune fallow's dead-code rules (#1363)
Evaluated knip (webpro-nl/knip) against the fallow setup already in the
repo, cleaned up everything it surfaced, then removed knip again: measured
head-to-head on the same tree, fallow is a strict superset once two
switches it already supports are flipped.
Dead code removed:
- `daemon/artifact-materialization.ts` (224 lines) had no production
caller, only its own test. Removing it exposed that
`downloadArtifactToTempDir` and the whole URL-fetch-with-redirects path
in `artifact-download.ts` were reachable only through it — the live
upload paths use the incoming-request helpers instead. That file goes
348 -> 123 lines. `readZipEntries` then fell out of `artifact-archive.ts`.
- Dead test-helper exports: 12 unused re-exports and 6 needlessly-exported
mocks in `session-test-harness.ts`, dead barrel entries in
`__tests__/test-utils/index.ts`, plus `withMockedXcrun`, `matchesSchema`,
`IOS_FRAME`, `IOS_TAB_FRAME`, `snapshotWithOffscreenContent`.
- `androidSnapshotHelperOutput` was duplicated byte-for-byte in
`provider-scenarios/android-world.ts`; it now imports the shared copy.
- 8 unreferenced type aliases, and 17 redundant type re-export lines in
`client/client-types.ts`. The published `.d.ts` is byte-identical before
and after all 19 files: those types already reach consumers through
`contracts/*` via `CommandResult<...>`, so this is not an API change.
Tooling:
- `.fallowrc.json` gains `includeEntryExports`,
`ignoreExportsUsedInFile: {type, interface}` and `unused-types: warn`.
That combination is what made the findings above visible; the previous
config was quiet mainly because of its own suppression list.
- Dropped 2 now-obsolete `ignoreExports` suppressions, added 3 documented
ones (published `sdk/*` surface, tool-config `default` exports, and the
`AssertTrue<...>` totality guards that exist only to satisfy
`noUnusedLocals`).
`unused-types` stays at `warn`: 72 pre-existing type re-export lines across
27 files remain, tracked separately. Every other fallow detector is at zero.
|
||
|
Michał Pierzchała
|
c84b449ecc |
fix(android): stamp status-bar chrome during the walk instead of reconstructing it downstream (#1319) (#1359)
* test(settle): pin that an expanded quick-settings shade stays visible to --settle (#1319)
#1319 asked whether the systemui run-condemnation rule leaves `--settle` blind
to a fully expanded quick-settings shade, the way it blinded the replay
divergence in #1318. It does not, and this pins the reason.
The settle loop captures `interactiveOnly: true` (`stable-capture.ts`). That
walk drops the structural systemui window spine (`legacy_window_root`,
`notification_panel`, `qs_frame`, the quick-settings ComposeView chain) — and
those are exactly the nodes that merge the shade into ONE contiguous run in the
`--raw` / non-raw shape #1318 measured. Under settle's shape the same capture
arrives as five runs, only `split_shade_status_bar` carries a marker, and the
29 quick-settings nodes survive into both diff sides.
So settle and divergence do not read one tree differently, as #1318 framed it:
they consume different capture shapes, and settle already gets the outcome that
layer wants — shade content diffs, status-bar churn does not.
Separately, the quiet-detection loop digests the UNFILTERED capture
(`digestSnapshotNodes` in `stable-capture.ts`), so the shade would reset the
quiet window even in the hypothetical where chrome stripping had emptied the
diff. Both halves of the question come out clean.
No product change. The behavior is correct but rested on an untested structural
coincidence: retaining the systemui spine in the interactive walk would re-merge
the runs and make `--settle` report a full-cover shade as bare removals with no
added content and no hint. The test fails if that happens (verified by flipping
the helper to `interactiveOnly: false`).
Verified live on emulator-5554 (Pixel 9 Pro XL API 37, deskclock), both
directions #1319 asked about and both halves the test asserts:
- shade OPENING mid-settle: settled after 6917ms: +28 -25
(added: brightness seekbar, Wi-Fi/Bluetooth/Mobile data/Quick Share/
Modes/Wallet tiles)
- shade CLOSING mid-settle: settled after 6919ms: +25 -28 (mirror image)
- the shade's own status bar ("Tue, Jul 21, Wifi signal full., T-Mobile") is
absent from both settle diffs while `snapshot -i` of the identical screen
still lists it — the run rule filters the churn rather than sitting inert.
The archived #1318 capture run through the real interactive-only walk reproduces
the live tree node-for-node (35 nodes, 29 kept, 6 stripped).
* docs(android): correct the chrome-classifier TODO — window-type keying is ruled out on device
Follow-up to the #1319 investigation, acting on review feedback that a
paragraph justifying fragile behavior means the code is wrong.
Two candidate fixes for the capture-shape-dependent chrome classification were
tested, and BOTH are dead. Recording that here so the next person does not
re-walk them:
1. The window-type approach this file's own TODO proposed. Measured against the
live helper XML (emulator-5554, Pixel 9 Pro XL API 37): systemui reports
`window-type=3` (TYPE_SYSTEM) both collapsed and expanded, `TYPE_STATUS_BAR`
(2000) never appears, the helper stamps window metadata on window ROOTS only
(1 of 169 nodes in an expanded-shade capture), and an expanded shade is ONE
window hosting the status icons AND the quick-settings tiles. No window-level
signal separates them. The TODO promised a fix the device data rules out.
2. Replacing run-condemnation with "condemn each marked node's subtree plus
fully-condemned ancestors". Shape-independent as intended, and it keeps the
tiles in both walks — but on the COLLAPSED status bar it leaks the unmarked
chrome the walk re-parents next to the markers ("Battery 100 percent.", the
notification-icon summary, neither carrying any resource-id). That is the
ticking-clock regression #1319 explicitly warned against.
So run-condemnation is not a lazy workaround: it reconstructs identity the walk
already discarded when it drops the `status_bar*` wrappers and re-parents chrome
leaves next to content. That upstream information loss is the actual root cause,
and a provenance-preserving walk is the real fix — larger than this PR, and it
would let #1318's divergence fallback be revisited.
Also trims the #1319 test's doc comment from a long justification of the current
behavior down to the finding, the defect, and what the test holds in place.
* fix(android): stamp status-bar chrome during the walk instead of reconstructing it downstream
Chrome classification gave opposite answers about the same screen depending on
capture shape: an expanded quick-settings shade was 100% chrome under the
`--raw`/non-raw walk (#1318 — every tile condemned, needing a divergence-local
fallback) and ~17% chrome under the interactive-only walk (#1319). Two layers
were papering over one broken classifier.
Root cause: `shouldIncludeStructuralAndroidNode` drops the `status_bar*` /
`navigation_bar*` containers — the only nodes that identify the region — and
re-parents their leaves next to real content. Everything downstream was
reconstructing identity the walk had already discarded, and reconstruction is
what depended on shape.
Fix: record it while it still exists. `walkUiHierarchyNode` threads
`ancestorSystemChrome` exactly like the `ancestorHittable` it already carries,
and stamps `systemChrome` on the emitted node; `androidUiNodes` tracks the same
subtree for the streaming content-recovery pass. Classification is then per node
and intrinsic, so `--raw` and non-raw agree by construction.
This deletes what was compensating for the loss:
- the 18 hand-picked marker leaf ids and their justification comment (what
enumerates them is "descendant of a status/nav-bar container" — now stamped);
- `collectAndroidSystemChromeRunIndexes`, the run-condemnation rule that let one
`clock` node condemn 95 unrelated ones;
- the leaf-id/prefix split, replaced by one container predicate that matches
`status_bar`/`navigation_bar` as an id SEGMENT so the shade's own
`split_shade_status_bar` counts.
Net −53 lines across 8 files, almost all of it classifier and prose.
Two alternatives were measured and rejected before this one (both recorded in
#1319 so they are not re-attempted):
- keying off AOSP window types, which this file's own TODO proposed:
impossible. On a live device systemui reports `window-type=3` (TYPE_SYSTEM)
collapsed AND expanded, `TYPE_STATUS_BAR` never appears, metadata is stamped
on window roots only (1 of 169 nodes), and an expanded shade is ONE window
holding the status icons and the tiles.
- condemning marked subtrees plus fully-condemned ancestors: leaks the unmarked
chrome the walk re-parents beside the markers ("Battery 100 percent.", the
notification-icon summary) — the ticking-clock regression #1319 warned about.
Test fixtures that modelled the old mechanism now model the device instead: the
synthetic status bar gets the `status_bar_launch_animation_container` root the
real capture has, and the content-recovery XML nests its chrome leaves inside
their container. Assertions were not weakened — settle.test.ts stamps via the
production predicate, and the #1319 test now asserts both walks classify
IDENTICALLY, which is the property that was missing.
Verified live on emulator-5554 (Pixel 9 Pro XL API 37, deskclock):
- shade opening mid-settle: settled after 7655ms: +28 -25, tiles present,
zero status-bar chrome (unchanged from before — settle was already right)
- ordinary action, no shade: +1 -1, no chrome leak
- real captures: collapsed status bar 9/9 chrome; expanded shade keeps every
tile under BOTH walks, which is the behavior that changed
* fix(android): keep systemChrome provenance out of published nodes; simplify androidUiNodes
Review blockers on
|
||
|
Michał Pierzchała
|
b1b26f3b6a |
feat: publish scripts from active sessions (#1357)
* feat: publish scripts from active sessions * test: cover save-script force retargeting * fix: address active publication review findings |
||
|
devin-ai-integration[bot]
|
1f042c3d8d |
refactor(mcp): extract reference-pin state into tool-ref-pins module (#1344) (#1345)
* refactor(mcp): extract ref-pin state into focused tool-ref-pins module Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mcp): add success-path ref-pin wiring test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): narrow tool-ref-pins result types and replace as-cast with type guard Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): remove unnecessary as-casts in tool-ref-pins and command-tools Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): align ref-pin result handling with #1343 typed result projection Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): upstream types for ref-pin module (#1345) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mcp): introduce honest public targetKind-discriminated interaction response contracts Replace the public AgentDeviceClient return types for press/click/fill/longpress/find with serialized-payload-shaped response data (targetKind, flat ref/selector/x/y, per-command extras) instead of internal runtime result types. The internal PressCommandResult/FillCommandResult/LongPressCommandResult keep their kind/target shapes for the daemon runtime; the public CommandResultMap now points to the new response contracts. - Add PressCommandResponseData/FillCommandResponseData/LongPressCommandResponseData/FindCommandResponseData in src/contracts/interaction.ts. - Update CommandResultMap and command-result tests. - Add client-facing shape tests asserting the public response data discriminates on targetKind and exposes flat identity fields. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(contracts): include cost and iOS Maestro fallback fields in public response contracts Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
e7c02a9f4c |
feat: add advisory device claims (#1329)
* feat: add advisory device claims * fix: preserve advisory claim ownership * fix: retain claims after incomplete cleanup * fix: retain claims across pre-open effects |
||
|
Michał Pierzchała
|
a6711a46cb |
fix(replay): publish a full-cover system overlay's targets in divergence screen.refs (#1318)
* fix(replay): publish a full-cover system overlay's targets in divergence screen.refs A fully expanded quick-settings shade left the divergence `screen` available but empty: 95 captured systemui nodes, 0 refs, no suggestions. `selectDivergenceScreenRefNodes` drops `collectSettleChromeRefs` nodes before `isForeignOverlayDismissTarget` can rank them, and the run-level chrome rule condemns a whole contiguous same-package run when any node in it carries a status/nav marker. An expanded shade is a SINGLE systemui run: the 4 status-bar icons it hosts (clock, mobile_combo, mobile_signal, wifi_signal) condemn all 95 nodes, the 23 hittable qs_tile targets included. The run rule assumes the status bar is its own window — true collapsed, false expanded. Since #1301 a plain `snapshot` of that same surface returns the tiles (the `systemSurfaceOnly` carve-out), so the divergence had become strictly NARROWER than `snapshot` — the invariant `captureDivergenceObservation` documents and the ADR 0012 decision 4 amendment forbids: the chrome filter must stay a FILTER, never a narrower scoping. Fixed in the divergence layer, not in the shared run rule: settle and divergence genuinely disagree about these nodes and both are right. Settle must keep stripping the shade run — the markers are literally clock/signal/wifi, the churn settle exists to ignore, and sparing runs that hold actionable content would let a ticking clock hold settle awake. The divergence layer owns the violated contract. Chrome exclusion now falls back to hittable chrome nodes when it would otherwise empty the pool, mirroring the existing `covered` fallback. The gate is hittability, not label presence, and both real captures back that: the collapsed status-bar clock is labeled "7:03" but not hittable (so a chrome-only screen still publishes nothing), while the expanded shade's clock is hittable. Live-verified on emulator-5556 (Pixel 9 Pro XL API 37): same repro 0 -> 20 refs (cap, truncated) — the brightness slider plus 19 hittable tiles, matching the fixture-driven test exactly. `ANDROID_QS_SHADE_CAPTURE_RAW_NODES` is a real 138-node --raw capture, not hand-authored ids. * docs(replay): state the chrome-fallback boundary (PR #1318 review) The fallback fires only when chrome exclusion would empty the pool, mirroring the `covered` fallback right below it. A marker-bearing overlay over PARTIALLY visible app content keeps its tiles condemned — the pool is non-empty, so no fallback. That is deliberate (the full-cover case is the one that strands an agent), but it was implicit; stating it saves the next investigator a live session. * refactor(test): move Android capture fixtures into .json files The fixture module had grown to 1080 lines, of which ~990 were two inlined device-capture literals — the walkers and their docs were buried under the data they operate on, and any future capture would bury them further. Capture fixtures are archived device trees: DATA to be regenerated from a device, not code to be hand-edited. They now live in `.json` beside the module and load via `fs` + `import.meta.url` — the existing `test/output-economy` baseline pattern, which needs no `resolveJsonModule` or import-attributes support to typecheck, and keeps the trees out of the bundle. Mechanically extracted from the current exports and verified byte-identical (`JSON.stringify` before/after match for both fixtures), so this is a pure move: no fixture data changed. Module drops 1080 -> 109 lines. The leg-E regression test remains revert-sensitive. * refactor(test): typecheck the capture fixtures via resolveJsonModule Reading the fixtures with `fs` + `JSON.parse(...) as RawSnapshotNode[]` bought nothing over a static import and gave up the one thing that matters for a typed data fixture: the cast asserts the shape away unchecked, so a fixture that drifts from `RawSnapshotNode` compiles fine and only surfaces as a confusing test failure — or silently passes. `resolveJsonModule` + `import ... with { type: 'json' }` structurally checks each tree against `RawSnapshotNode[]` at typecheck time instead. Verified: corrupting a fixture (`hittable: "yes-please"`, a stray field) now fails `tsc` at the export, where the `fs` version accepted it. Also drops the reader helper and the runtime read. The `test/output-economy` precedent I first copied reads baselines through `fs` for a different reason — those are runtime-compared artifacts, not statically typed fixtures — so it did not apply here. tsconfig gains one option; `noEmit` is already set, so it has no output-layout effect. Fixture data unchanged (byte-identical), leg-E test still revert-sensitive, full tooling + 1931 tests green. * refactor(replay): inline the chrome fallback instead of explaining it `hittableChromeCandidates(nodes)` did not filter chrome — it returned every hittable node, and was only "chrome candidates" by virtue of its one call site. The name lied, and a chunk of the 15-line comment above it existed to cover the gap between the name and the behaviour: exactly the case where the comment is propping up the code. Inlined into the same `x.length > 0 ? x : y` idiom the `covered` fallback two lines below already uses, so the two narrowings now read in parallel. The boundary that comment spelled out — the fallback fires ONLY when exclusion empties the pool, so a partial-cover overlay keeps its tiles condemned — is now the ternary itself rather than a paragraph asserting it. What survives is the part the code genuinely cannot show: WHY a chrome-only screen must still publish (the never-narrower-than-`snapshot` contract) and why hittable is the discriminator. 6 lines, against the 8-line `covered` comment beside it. No behaviour change: same node set, fixtures untouched, leg-E test still revert-sensitive, 1931 tests + full tooling green. |
||
|
Michał Pierzchała
|
9b5f333a25 |
fix(cli): deliver --no-record to the daemon (supersedes #1305) (#1311)
#1305 claimed to forward --no-record from "every recordable command reader". Measured through the real argv -> reader -> client -> daemon chain on its own merge commit, the flag reached the daemon for ONE command (`open`). It is now 5 of 33 on current main -- `open` plus get/is/find/snapshot, the latter four only incidentally, because #1303 declared `noRecord` in their metadata. #1305's fix was inert because it fixed a layer that is not load-bearing. Its test asserted on `readInputFromCli` output -- an intermediate object two later layers rebuild from scratch: 1. `defineExecutableCommand.invoke` runs `metadata.readInput(input)` -> `readFieldInput`, which keeps ONLY declared metadata fields plus `readCommonInput`'s output. `noRecord` was neither, so it was filtered. (`open` survived solely because its metadata declares the field.) 2. Each `to*Options` projection rebuilds the client options from `commonToClientOptions` plus its own named fields; that helper did not carry `noRecord` either. So `--no-record` parsed, was accepted on every command, and was silently dropped before dispatch -- including on press/click/fill and, per the maintainer's review, gesture/back/home. Fixed at the seams the flag must survive, not per reader: - `commonInputFromFlags` and `selectionOptionsFromFlags` (the reader layer has TWO parallel common helpers -- reader-input shape vs client-options shape -- so both must carry it; `settings` used only the latter, which is why it was the last gap); - `readCommonInput` (stop `readFieldInput` filtering it); - `commonToClientOptions` (stop `to*Options` dropping it). Measured after: 33/33 deliver the flag, with zero hand-listed commands. #1305's `noRecordInputFromFlags` helper and all 13 hand-added call sites are deleted: they are redundant against the seams, and leaving both would be two sources of truth for one behavior -- exactly how the next gap breeds. Preserves the --record asymmetry (ADR 0012 decision 6 amendment): --no-record is common and rides the common seam; --record stays scoped to snapshot/get/is plus a dynamically-validated find, on its own narrow helper. Also fixes `get --record`, dead through the CLI since #1303 for the same re-projection reason (`toGetOptions` rebuilds its options object), which that PR's daemon-level scenario could not see. Coverage is asserted where it is observable, not at the intermediate object: - `cli-record-flag-delivery.test.ts` drives real argv and asserts on the DAEMON REQUEST for all 32 recordable routes; it fails on reverting either seam ("press accepted --no-record but never delivered it to the daemon"). - `no-record-recorder-routes.test.ts` is a healed-script regression: gesture/ back/home with --no-record must not land in a written .ad. Reverted, it fails with the leaked `gesture "fling" "up" 100 200` line in the script. A derived `recordsSessionAction` classification + completeness gate follows in a separate PR: this fixes the 32, that makes a 33rd impossible. |
||
|
Michał Pierzchała
|
c245906b70 |
fix(cli): forward --no-record from every recordable command reader (#1304) (#1305)
`--no-record` is accepted on every command (its key is in
COMMON_COMMAND_SUPPORTED_FLAG_KEYS, which seeds every command schema's
supportedFlags) and is documented as "Do not record this action", but no
interaction/capture reader forwarded it into the options object the daemon
request is built from. The flag parsed, then vanished.
The rest of the chain was already wired: command-flags.ts maps
options.noRecord onto the request, and recordActionEntry reads
entry.flags?.noRecord to skip the action. Only the reader step was missing,
so the documented behaviour never happened for any of press, click, fill,
longpress, swipe, focus, type, scroll, get, is, find, snapshot, or wait.
`app` was the sole command that forwarded it.
Measured through the real argv path (parseArgs -> readInputFromCli), before
this change: 13/13 commands accept `--no-record`, 0/13 reach the options
object. After: 13/13.
Fixed at the shared seam rather than per reader. noRecord is a common flag,
so it gets a recordControlInputFromFlags() helper next to the existing
settleInputFromFlags/repeatedInputFromFlags group helpers, and each
recordable reader spreads it. A future reader picks it up by spreading one
helper instead of re-deriving a flag it never names.
The regression test covers all 13 recordable commands and fails without the
fix ("press dropped --no-record").
|
||
|
Michał Pierzchała
|
117f78107e |
feat: add direct Limrun provider runtime (#1278)
* feat: add direct Limrun cloud runtime * refactor: reuse Android provider runtime for Limrun * refactor: pass runner context to provider runtimes * fix: remove Android gesture swipe fallback * fix: reconcile Limrun direct runtime with main * refactor: compose Android provider interactors in core * fix: satisfy packaged Limrun runtime checks * perf: load Limrun provider runtime on demand * docs: document Limrun device cloud flow * refactor: reuse Android reverse provider for Limrun * fix: isolate provider-owned iOS sessions * fix: preserve provider runtime boundaries * refactor: split close repair lifecycle * fix: reject unavailable provider leases * fix: reconcile provider runtime review feedback * test: stabilize alert deadline smoke assertion * fix: recover expired provider leases * fix: limit Limrun to remote simulators * fix: make Limrun provider cleanup durable * test: cover Limrun connect through CLI * fix: make provider expiry recovery durable * refactor: remove Limrun compatibility cleanup * fix: release live provider leases on expiry |
||
|
devin-ai-integration[bot]
|
668b64f3da |
chore: remove deprecated rotate CLI command alias (#1277) (#1283)
* chore: remove deprecated rotate CLI command alias (#1277) The rotate CLI command alias was renamed to orientation a few versions ago and is now removed at the next minor, aligned with the gesture-shim deprecation window. - Remove the rotate -> orientation entry from src/cli-command-aliases.ts. - Add an actionable parser error: invoking rotate now fails with "rotate was renamed to orientation". - Update the command-suggestion guard comments and the true-alias list in the curated suggestion map test. - Update CLI parser/help usage tests and src/__tests__/cli-help.test.ts to assert the migration error. - Remove the rotate deprecation note from the commands doc and add a breaking migration note to the changelog. No other command-name aliases are marked deprecated; long-press, metrics, tap, launch, and relaunch remain supported true aliases. Fixes #1277 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: disambiguate removed rotate alias error message (#1277) Update the migration error so users who meant the two-finger gesture are pointed to `gesture rotate` instead of `orientation`. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: align CHANGELOG with runtime rotate migration message (#1277) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
e58cbcdb5f |
refactor: colocate native platform sources under android/, apple/, linux/ (#1273)
Move the scattered root-level native projects into per-platform folders and drop
the now-redundant platform prefix:
- android-ime-helper/ -> android/ime-helper/
- android-multitouch-helper/ -> android/multitouch-helper/
- android-snapshot-helper/ -> android/snapshot-helper/
- apple-runner/ -> apple/runner/
- macos-helper/ -> apple/macos-helper/
- src/platforms/linux/atspi-dump.py -> linux/atspi-dump.py
Only repo source paths move. Identity surfaces stay frozen so no user's runner
cache is invalidated on upgrade: the derived-cache key hashes source paths
relative to AgentDeviceRunner and excludes packageVersion, and the
~/.agent-device/{apple-runner,macos-helper} namespaces, the
agent-device-android-*-helper artifact/manifest/protocol names, the
AgentDeviceRunner Xcode project, and the `prepare ios-runner` CLI command are
unchanged. Updates build/package scripts, CI, package.json files+scripts,
ignore/attr/fallow configs, runtime path resolvers, and test fixtures.
Also: re-base repo-root-relative refs inside the moved apple/runner for the
added nesting level (gated XCUITest fixture walk + two doc links), and clean the
legacy dist/apple-runner packaged output so the relocated runner can't
double-ship into the wholesale-included dist (with a regression test).
|
||
|
Michał Pierzchała
|
37895caf99 |
refactor: replace Maestro compat with typed direct engine (#1217)
* test: add pinned Maestro conformance harness * feat: add typed Maestro program IR parser * docs: define direct Maestro engine architecture * test: compare Maestro oracle with typed IR * feat: add direct Maestro program engine * refactor: narrow Maestro execution context * refactor: tighten Maestro program parsing * fix: verify iOS Maestro visibility waits * refactor: isolate retained Maestro runtimes * refactor: type Maestro target resolution * refactor: harden typed Maestro execution * refactor: share in-page swipe planning * feat: add typed Maestro runtime port * refactor: parse Maestro suite metadata from typed IR * refactor: centralize Maestro include loading * feat: execute Maestro files through typed engine * refactor: share replay built-in variables * fix: make Maestro target intent explicit * fix: refresh Maestro targets before input * refactor: format Maestro progress from typed IR * feat: compile typed Maestro replay plans * feat: bind typed Maestro runtime to public commands * feat: route Maestro YAML through typed runtime * refactor: remove legacy Maestro runtime * refactor: remove obsolete replay control model * refactor: split typed Maestro plan modules * fix: harden typed Maestro runtime semantics * docs: update direct Maestro architecture * fix: reconcile Maestro runtime with merged contracts * fix: harden typed Maestro execution boundaries * fix: harden typed Maestro runtime evidence * perf: avoid eager Maestro device resolution * refactor: finalize typed Maestro execution * fix: reject Android system-only helper snapshots * fix: preserve Android system dialog snapshots * fix: make helper-backed CI deterministic * refactor: invalidate Maestro observations before dispatch * fix: make Maestro selector policy explicit * refactor: remove Maestro ranking sentinels * refactor: make Maestro own observation stabilization * refactor: source Maestro compatibility presets * refactor: keep Maestro failure reports typed * refactor: simplify Maestro runtime policy * fix: isolate Maestro engine failures * refactor: consolidate Maestro swipe presets * fix: align Maestro selector and observation semantics * fix: preserve atomic iOS Maestro taps * fix: require semantic uniqueness for Maestro taps * fix: preserve Maestro parse provenance * docs: pin Maestro compatibility presets * docs: reconcile Maestro gesture viewport contract * perf: resolve Maestro gesture viewport directly * test: align Maestro replay regressions * fix: order Android gesture lift after endpoint * fix: settle Maestro gestures before continuation * fixup! fix: order Android gesture lift after endpoint * refactor: normalize Maestro swipes once * refactor: fail impossible Maestro observations * refactor: normalize Maestro defaults alias * test: reconcile Android provider scenarios * fix(android): synchronize single-pointer move events * test: align repair digest parsing * refactor: type Maestro runtime operations * refactor: keep Maestro controls compact * refactor: name Maestro diagnostic limit * fix: align Maestro parser and settling semantics * fix: complete Maestro compatibility semantics * docs: define Maestro compatibility boundaries * fix: refresh iOS runner target after relaunch * fix: reset prewarmed iOS runner after URL open * fix: preserve iOS Maestro target and swipe intent * fix: harden direct Maestro runtime semantics * fix: preserve ranked Maestro replay suggestions * fix: align maestro tap runtime semantics * fix: stabilize maestro ci contracts * fix: tighten maestro runtime architecture * fix: reconcile maestro replay with latest main * perf: tighten Maestro iOS stabilization * fix: preserve Maestro app lifecycle sessions * fix: restore Maestro CI coverage * fix: address Maestro engine review findings * refactor: consolidate Maestro compatibility internals * fix: scope Maestro target evidence to childOf |
||
|
Michał Pierzchała
|
95c1e201cb |
feat: --force/--overwrite for --save-script + arm-time EEXIST preflight (#1266)
* feat: add --force/--overwrite for --save-script, arm-time EEXIST preflight #1235 made healed-script publication refuse-on-exist. #1258 adds an escape hatch: --force (alias --overwrite) on open/close/replay makes publishHealedScriptAtomically atomically REPLACE an existing target (renameSync) instead of refusing. The flag threads CLI -> daemon request -> SessionState.saveScriptForce (persisted at arm time, like saveScriptPath, so a later close/auto-commit that doesn't repeat the flag still honors it) -> the publish primitive. Default (flag absent) is unchanged: refuse-on-exist. Second half: an arm-time EEXIST preflight in session-replay-runtime.ts now fails a repair-armed replay --save-script BEFORE any step dispatches when its target already exists and --force is not set, instead of only failing at publish time after the whole repair run (and its corrective steps) has already executed against the device. * refactor: extract write() catch-block into handleSessionScriptWriteFailure Pure structural refactor, no behavior change: moves the diagnose + classify-and-return / AppError-rethrow logic out of SessionScriptWriter.write into a module-level helper. Drops write()'s cyclomatic below Fallow's threshold (the finding CI flagged on #1266) — the extracted throw still propagates out of the catch exactly as before. * fix: make persisted save-script force consistent (preflight + per-target) Addresses two authorization inconsistencies in the #1258 --force work flagged in re-review: 1. Arm-time EEXIST preflight now uses the SAME effective force decision as publication — `req.flags?.force || preRunSession.saveScriptForce` — instead of the live flag alone. A repair armed with `--save-script --force` and continued via `replay --from … --save-script` (without repeating --force) is no longer rejected on a target the earlier forced leg authorized. 2. Force is now per-target, not sticky-across-retarget. Re-arming a DIFFERENT `--save-script=<other>` without a live --force CLEARS the persisted `saveScriptForce` (new shared `applySaveScriptRetarget`, used by both the replay armer and `recordActionEntry`/close), so a retarget can never silently overwrite a file nobody opted into. A live --force on the retarget re-grants it for the new target. Updated the SessionState doc accordingly. Regressions: persisted-force `--from` continuation is not preflight-rejected; retarget-without-force refuses an existing target (and the --force contrast overwrites it). Both verified to fail without their respective fix. * fix: match arm-time preflight force to per-target retarget contract Re-review ordering-mismatch fix. The preflight computed effective force as `req.flags?.force || preRunSession.saveScriptForce`, accepting persisted force regardless of whether THIS request retargets. So a `--from` continuation with explicit `--save-script=b.ad` (no live force) on a session forced for a.ad would PASS the preflight, run every step, then have `applySaveScriptRetarget` clear the force for b.ad, and finally refuse the existing b.ad at publish time — defeating the arm-time-preflight point and mutating the session mid-run. Now the effective-force decision is computed inside `preflightSaveScriptTarget` against the target THIS request resolves to (the same one the armer will set), matching `applySaveScriptRetarget`'s per-target contract: live force always bypasses; persisted force bypasses ONLY when `targetPath === existingSaveScriptPath` (same target). A retarget to a different path without live force is now refused BEFORE any step dispatches. The preflight stays read-only (runs before the armer; never mutates the session). Regression: a --from continuation retargeting to an existing b.ad without live force fails at the preflight, dispatches zero steps, and leaves saveScriptPath/ saveScriptForce unchanged. Verified it fails (leg passes preflight, runs) when the fix is reverted. Same-target continuation and live-force retarget stay correct. * fix: expose close savedScript to clients; preserve COMPLETE on retarget reject Two re-review blockers. BLOCKER 1 (client-contract gap): the typed close APIs accept --save-script/ --force inputs but dropped the daemon's `savedScript` response, so a Node client could request publication but not learn where the file landed. Add `savedScript?: string` to SessionCloseResult and AppCloseResult, and project it via readOptionalString in both close normalizers (sessions.close, apps.close). New public-client coverage asserts it round-trips (and is absent when the daemon published nothing). BLOCKER 2 (P1 ordering): the C2 `saveScriptComplete = false` reset ran BEFORE the EEXIST preflight's early-return, so a retarget REJECTION corrupted a prior COMPLETE transaction — a later close would then refuse to commit the original target. Move the reset to AFTER `if (saveScriptPreflight) return ...` (verified nothing between the two positions reads saveScriptComplete; applySaveScriptRetarget already runs later, so the original saveScriptPath is preserved). The reset is correct only when the run proceeds to re-arm. Strengthened the retarget-rejection regression to assert (a) saveScriptComplete survives the rejection and (b) a later bare close commits the ORIGINAL target, not the rejected new one. Both verified to fail without their respective fix. |
||
|
Michał Pierzchała
|
54977f3b87 |
feat(daemon): ADR 0014 session ref-frame lifetime — full implementation (#1257)
* feat(daemon): classify ref-frame effect on every daemon command (ADR 0014 step 2) Add the ADR 0014 `refFrameEffect` trait to the daemon command descriptor facet: every command that reaches a session-owning daemon leaf declares how it relates to the session's authorized ref frame — `preserve`, `may-invalidate`, `delegated`, or a request-sensitive resolver for subaction-dependent commands (keyboard status vs dismiss, alert get/wait vs accept/dismiss). This is the honesty/completeness guard, not the transition site: a `may-invalidate` command still calls the (future) ref-frame module only when its mutating path runs. No runtime behavior changes here. - `RefFrameEffect` / `DaemonRefFrameEffect` types and a `resolveRefFrameEffect` accessor honoring the resolver form, mirroring the existing closure traits. - Classify all 58 daemon-faceted commands; `find` is the honest superset (`may-invalidate`) pending a read/mutate resolver during enforcement wiring. - Give `app-switcher` a daemon facet (route unchanged) so the generic-fallback escape hatch the ADR calls out is covered instead of silently unclassified; drop it from parity's UNROUTED set. - Completeness gate (`ref-frame-effect.test.ts`): every daemon-projected command classifies an effect, every public command is classified or in the explicit non-daemon allowlist (`install-from-source`, which projects via the `install_source` internal command), and the resolvers/app-switcher resolve as declared. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): introduce ref-frame module + admission matrix (ADR 0014 step 1) Introduce `src/daemon/ref-frame.ts` as the single owner of the ADR 0014 ref-frame model — the authorization namespace for mutation refs, kept distinct from the latest operational observation (`session.snapshot`). It defines the frame's issuance scope and lifecycle state and the pure mutation-admission matrix (`admitRefMutation`) with the ADR's typed, order-sensitive reasons: ref_frame_expired, ref_generation_mismatch, plain_ref_requires_complete_frame, ref_not_issued. The frame is introduced behind the existing `snapshotGeneration` (epoch) and `snapshotRefsStale` (coarse client-stale) fields, whose wire-visible names (`refsGeneration`, the `@e12~s42` pin grammar) are unchanged. New `refFrameState`/`refFrameScope` session fields default to active/all, so the matrix currently reduces to the generation-pin check the iOS path already did — no behavior change. Expiration at the side-effect seam and non-`all` scope land in later steps. The existing #1241 iOS stale-ref guard now routes its decision through `admitRefMutation` (plus the transitional coarse-stale check for plain refs), so the module is production-live; the external error contract is identical. Adds a unit test covering the full admission matrix and reason ordering. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): wire pre-side-effect frame expiration at the seams (ADR 0014 step 3) Route device mutations through the idempotent ref-frame transition. A leaf expires the current frame synchronously, immediately before awaiting the device operation, so success, timeout, cancellation, or connection loss all leave it expired — there is no success-only rollback. Seams wired: - interaction runtime backend closures (tap/click, fill, longPress, native web clickRef/fillRef, gesture, type) — post-resolution, pre-dispatch, so a resolution failure before the seam preserves the frame; - the generic daemon leaf (back/home/rotate/scroll/tv-remote/app-switcher/ viewport/focus, ...), gated by the daemon `refFrameEffect` classification via `resolveRefFrameEffect`, which is that resolver's first production consumer. Re-authorization: issuing a complete namespace re-activates the frame — `markSessionSnapshotRefsIssued` and the snapshot command's `buildNextSnapshotSession` — so a fresh capture between mutations restores usability. A diff or kept tree preserves the prior authorization state; internal read captures never re-authorize. Enforcement of the new expired-frame rejection is intentionally deferred to step 7, which the ADR gates on fresh live device evidence per platform. The iOS #1239 guard therefore stays armed-but-not-enforced here: it consults the admission matrix but still rejects only on the pre-existing conditions (pinned generation mismatch, coarse plain-ref stale marker), so behavior is unchanged. Tests prove the transition is wired (a press expires the frame; a re-issue re-activates it) alongside the idempotency and re-authorization unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix(daemon): address ADR 0014 review — partial issuance, keyboard, seam coverage Exact-head review found three blockers; all fixed with focused seam tests. 1. Partial issuance no longer restores complete authority. Every caller of `markSessionSnapshotRefsIssued` (find, settled diff, replay divergence) is a PARTIAL publication, but it re-activated a complete `all`-scope frame. It now only clears the coarse marker; complete re-authorization is reserved for the snapshot command (`activateCompleteRefFrame`, from `buildNextSnapshotSession`). 2. Keyboard resolver covers every mutating subaction. keyboard accepts status/get/dismiss/enter/return; only status/get read, so dismiss/enter/return (enter/return dispatch a real return key) are now `may-invalidate`. Alert reads are likewise a named set. Completeness test extended. 3. Remaining step-3 leaf seams wired: the direct iOS selector fused dispatch, the direct `find` focus/type dispatches (find click/fill already delegate through the interaction leaf), and Android blocking-dialog recovery (expire before the recovery tap). Focused seam tests for each prove the frame expires. Enforcement of the expired-frame rejection remains deferred to step 7 behind the ADR's per-platform live-evidence gate; behavior is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): cross the seam at every specialized mutating leaf (ADR 0014 step 3 complete) Wire expireRefFrame at the remaining may-invalidate leaves so EVERY mutating daemon leaf crosses the side-effect transition, not just the interaction/generic paths: - keyboard dismiss/enter/return, push, trigger-app-event (shared session leaf) — gated by resolveRefFrameEffect so keyboard status/get preserve the frame; - alert accept/dismiss (get/wait preserve, via the alert resolver); - settings mutations; - React Native overlay dismissal; - install / reinstall (deploy op); - open / relaunch — expires the reused session's frame before the launch; - close — expires for uniformity, though a successful close deletes the whole session (and its frame) anyway. Seam tests: keyboard dismiss expires while status preserves (proves the resolver-gated pattern), and RN overlay dismissal expires. Enforcement stays deferred; behavior unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): partial issuance scope + MCP pin retention + pinned CLI refs (ADR 0014 step 4) A find/settled-diff/divergence result publishes only the refs it returned, so it now activates a bounded PARTIAL frame authorizing exactly those ref bodies (`markSessionPartialRefsIssued`) instead of nothing — a plain ref then requires a complete frame and a pinned ref outside the set is rejected. An empty partial result leaves prior authority intact. - read-only find publishes its one ref; settled diff publishes its added lines + `refs` + `tail`; divergence publishes its capped, non-covered, non-chrome digest set. - MCP: a mutating `find` returns no `refsGeneration` and is explicitly non-issuing — it no longer hits the missing-generation branch that wiped the whole per-session pin scope (forwarding the old pin is how the daemon produces a precise stale rejection). - Human-CLI partial results render reusable refs in ready-to-copy `@eN~s<gen>` form (find + settled tail); JSON/Node keep plain bodies + one response-level generation, and MCP stays plain (it auto-pins). Output-economy waiver covers the +8-byte tail-pin increase with an ADR justification; the workflow oracle treats a pinned ref as surfacing its plain body. Enforcement of the frame's expiry and partial-scope rejections stays deferred to step 7 (behind the ADR's per-platform live-evidence gate), so this is behavior-preserving; the iOS guard now consumes the admission verdict for a typed `details.reason` on the rejections it already emitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): resolve refs against the authorized frame tree (ADR 0014 step 5) Retain the ref frame's immutable source tree (shared reference, no deep copy) and resolve a `@ref` against it rather than the latest operational observation. An Android freshness — or any read-only — capture advances `session.snapshot` without disturbing the frame tree, so the two intentionally diverge. At resolution, adopt the fresh observation's node (its current on-screen coordinates) ONLY when its local identity still matches the authorized node — the legitimate "element moved" case. If a different element now sits at that index, keep the authorized frame node so a positional coincidence cannot retarget the action. Expose the frame tree to the command runtime through `CommandSessionRecord.refFrameSnapshot`; pre-frame sessions fall back to `snapshot` and behave exactly as before. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * feat(daemon): fail-closed ref-mutation enforcement across platforms (ADR 0014 step 7) Enforce the ref-frame admission matrix on every platform before dispatch: an expired frame, a superseded generation pin, a plain ref against a partial frame, or an unissued pinned ref is now rejected with a typed `details.reason` and an honest message that names the lifetime failure instead of claiming the ref was missing or lacked bounds. The prior iOS-only, coarse-marker guard is replaced. Freeze the frame epoch at issuance (`refFrameGeneration`) so a later read-only capture that advances the observation counter cannot falsely reject a correct pin from the issuing frame; staleness warnings compare against the same frame epoch. A mutating `find` re-resolves its target by locator against a fresh capture, so its internal leaf dispatch carries `internal.findResolvedTarget` and skips ref admission (it still crosses the seam and expires the frame). Update unit and provider-integration scenarios to the new contract: multi-mutation ref sequences re-observe between mutations, settled refs are consumed in pinned form, and rejections assert the typed reason. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * docs(adr-0014): promote ref-frame vocabulary and mark implementation status Flip ADR 0014 to Accepted, promote the ref-frame / frame-expiry-seam / mutation-admission vocabulary into CONTEXT.md, correct the `@ref` resolution note to the frame-tree model, record the migration status (steps 1–7 landed; coarse-marker removal follows live-evidence confirmation), update ADR 0012's divergence-ref amendment to accepted, and add a CHANGELOG entry for the fail-closed ref lifetime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * test(daemon): lock ADR 0014 evidence #1 and refresh module docs Add a daemon-level sequence test proving the canonical contract: after an unobserved first ref mutation, a second mutation rejects both bare and pinned with ref_frame_expired, and a fresh snapshot re-authorizes. Refresh the ref-frame module header and seam-expiry test comment now that enforcement is live. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix(daemon): address ADR 0014 exact-head review — six lifetime blockers 1. Android dialog recovery aborts an outstanding ref action: a ref press/fill admitted against the pre-recovery frame now fails with ref_frame_expired when before-command recovery mutates the UI, instead of continuing against the recovered screen (selector/coordinate actions still re-resolve and continue). 2. open --relaunch expires the existing session's frame BEFORE the close dispatch, so a close timeout/failure that already tore the app down still leaves the old frame expired. 3. expireRefFrame clears scoped-snapshot lineage (snapshotScopeSource) at the seam, so snapshot -s @ref -> mutation -> snapshot -s @same-ref can no longer borrow stale lineage across a device side effect. 4. Missing authorized-frame evidence fails closed: resolveSnapshotForRef no longer recaptures and accepts the same ref body from a newer tree by positional coincidence. A mutating find's internal dispatch resolves against its own fresh capture (omitRefFrameSnapshot), not the frame. 5. Mutating find omits refsGeneration — its acted ref is diagnostic pre-action identity and must not be pinnable after the action. 6. An empty partial publication leaves all session state untouched (including the coarse marker), instead of clearing it before finding there were no refs to issue. Adds focused regressions (lineage-cleared sequence, empty-partial no-op, fail-closed on unusable bounds, in-frame label recovery, mutating-find non-issuance) and extracts the find action dispatch to keep complexity in budget. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * fix: preserve snapshot refsGeneration + shared recovery rejection (ADR 0014 re-review) P1: structured JSON/Node snapshot results now retain the response-level refsGeneration. It was declared on the daemon response but dropped by the public CaptureSnapshotResult type, the serializer, and the Node normalizer, so default `snapshot -i --json` emitted refs with no generation to pin against. Added to the type, serializer, normalizer, plus CLI/Node tests. P2: Android dialog-recovery abort now reuses the SHARED admission rejection (refMutationAdmissionResponse) instead of a bespoke error, so the failure carries the full typed context (reason, ref, currentGeneration, scope, mintedGeneration) identical to every other expired-frame rejection across platforms. Removes the now-unused AppError/refFrameState imports. Adds a regression proving recovery aborts the outstanding ref action before any press dispatch. Also adds the relaunch failure-boundary regression (existing-session close fails after dispatch → old frame stays expired), and corrects the ADR implementation-status note so Android blocking-dialog recovery and a real provider-backed interaction/lifecycle are recorded as unexercised release blockers rather than confirmed enablement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * docs(adr-0014): record provider seam as live-verified; Android recovery sole blocker The provider-backed interaction + lifecycle seam is now confirmed by fresh live evidence (AWS Device Farm, webdriver backend). Update the ADR implementation-status note so only Android blocking-dialog recovery remains an unexercised release blocker — and note it is blocked on a bootable free Android target plus a deterministic app-owned ANR trigger, not on any code gap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa * docs(adr-0014): record Android ANR recovery as an accepted evidence gap Per the review decision: the Android blocking-dialog recovery seam has no deterministic app-owned ANR repro in the harness, so it was not live- exercised. The team accepted shipping without a live run for it — its transition/abort logic is covered by fixture regressions and it is enforced in code identically to the verified paths. Reword the status note from an open release blocker to a documented, accepted evidence gap, which unblocks step 8's coarse-marker removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BmnraQhvxzEFEUoUiqRpJa --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
392dc1cded |
refactor: rename rotate command to orientation (rotate kept as deprecated alias) (#1252)
* refactor: rename rotate command to orientation, keep rotate as a deprecated alias
The top-level `rotate` command (device orientation: portrait/landscape) shared
a name with the `gesture rotate` two-finger rotation gesture. Rename the
orientation command to `orientation` and keep `rotate` working as a minimal,
silent CLI alias (same mechanism as `tap`->`press`) for a few versions.
The rename is applied across every layer:
- command-descriptor registry `name`, daemon dispatch handler, and the typed
system facet (metadata/cliReader/daemonWriter/schema/output formatter)
- navigation projection + `CommandResultMap` (`OrientationCommandResult`,
`action: 'orientation'`), client types (`OrientationCommandOptions`), and the
runtime family (`device.system.orientation`)
- interactor + backend methods -> `setOrientation` (matching the backend's
`setKeyboard`/`setClipboard` verb convention); Android helper
`rotateAndroid` -> `setAndroidOrientation`
- Apple/cloud-webdriver capability keys and plugin gate
- user-facing docs (commands.md, client-api.md)
Client SDK method is `orientation` (client convention = camelCase of the
command name, matching `back`/`home`/`appSwitcher`); execution layers use the
imperative `setOrientation`.
Deliberately unchanged:
- the Swift runner wire protocol keeps `command: 'rotate'` — the runner has its
own command namespace with no gesture collision, so renaming it would only
risk CLI<->installed-runner version skew on physical devices
- the `DeviceRotation` value type / `parseDeviceRotation` (names the orientation
values, no collision)
Note: `client.command.rotate` / `device.system.rotate` and the `RotateCommand*`
exported types are removed (the alias only rewrites CLI tokens); SDK consumers
must use `orientation`. The JSON `action` value changes `rotate` -> `orientation`.
* style: wrap long lines to satisfy oxfmt (orientation rename tests)
* fix: add compatibility layer for the rotate->orientation rename
Addresses review blockers on the CLI-only alias: `rotate` previously
resolved only in CLI token parsing, so command-data/RPC paths that carry
the wire command directly failed descriptor validation, and the removed
typed SDK surface broke shipped consumers.
Central command-alias boundary (was CLI-only):
- Promote `cli-command-aliases.ts` to `command-aliases.ts` as the single
alias source, applied at each command-name ingress that bypasses the CLI
parser: the daemon request boundary (`handleRequest`, covering replay and
older remote clients) and the batch step readers (CLI `batch-steps.ts` and
daemon `batch-policy.ts`). No hand-synced command tables.
Retain deprecated typed SDK surface (shipped v0.18/v0.19):
- `RotateCommandOptions` / `RotateCommandResult` type aliases (legacy
`action: 'rotate'` contract) and `SystemRotate*` runtime types.
- `client.command.rotate` and `device.system.rotate` deprecated wrappers
that delegate to `orientation` and restore the legacy response
(`action: 'rotate'` / `kind: 'systemRotated'`).
ADR 0014: rename `rotate` -> `orientation` in the invalidation guidance
(lines 229, 237) so the accepted architecture doc matches the command name.
Tests: daemon-boundary rewrite, CLI+daemon batch alias resolution, and the
deprecated client/runtime wrappers preserving the legacy contract.
Live emulator evidence (emulator-5554):
- `orientation landscape-left` -> user_rotation=1
- `rotate portrait` (CLI alias) -> user_rotation=0
- batch step `{command:'rotate'}` (no CLI parser) -> user_rotation=1
* fix: preserve orientation rename compatibility
* test: stabilize orientation compatibility formatting
* style: format MCP compatibility test
* revert: drop cross-surface rotate compatibility, keep the lean rename
The rotate->orientation change is a bug fix (name collision with the
`gesture rotate` two-finger gesture), not a compatibility feature. The
cross-surface command-data compatibility added disproportionate weight
(~480 B, dominated by the alias module inlined into the batch bundle) for a
command that was only canonical for two minor versions, so shipped batch/
replay/MCP data carrying `rotate` is a rare, documentable break.
Removed:
- daemon request-boundary command normalization (`request-router.ts`)
- batch step alias resolution (`batch-policy.ts`, `cli/batch-steps.ts`)
- MCP tool-runner alias/legacy-result handling (`mcp/command-tools.ts`)
- the `command-aliases.ts` module rename and cross-surface machinery
(reverted to `cli-command-aliases.ts`)
- the cross-surface tests
Kept (cheap, high value — prevents build breaks for typed consumers):
- CLI `rotate` alias (one line, same mechanism as `tap`/`launch`)
- deprecated `RotateCommand*` / `SystemRotate*` type aliases and the
`client.command.rotate` / `device.system.rotate` wrappers that delegate to
`orientation` and restore the legacy response contract
Net bundle vs main is now +473 B (was +952 B), almost all the kept SDK
wrappers plus the unavoidable longer command name.
|
||
|
Michał Pierzchała
|
62fd46abd8 |
fix: Android status/nav-bar systemui chrome leaks into non-raw captures (#1251) (#1256)
* fix: recognize Android status/nav-bar leaf ids as systemui chrome (#1251) The non-raw Android walk (walkUiHierarchyNode/shouldIncludeStructuralAndroidNode in ui-hierarchy.ts) drops unlabeled/unidentified structural nodes, re-parenting their children upward. That silently swallows the status_bar*/navigation_bar* WRAPPER nodes carrying the marker ids collectAndroidSystemChromeRunIndexes keys on, leaving only their labeled/identified LEAVES (clock, battery, wifi/mobile icons, nav buttons) in a non-raw capture. Those leaves' own ids have no status_bar/navigation_bar prefix, so the systemui run loses its marker and is no longer dropped -- leaking status-bar chrome into --settle and replay divergence screen.refs. --raw keeps the wrapper markers, so it was unaffected. Recognize the surviving leaves directly by EXACT resource-id (not prefix, to stay tight -- actionable systemui overlays like the volume dialog or a media picker must keep surviving). Test derives a faithful non-raw tree from a real --raw Android capture (checkout-form fixture app, Gboard + status bar) by simulating the walk's drop+reparent for the specific marker-bearing wrappers, then asserts: every surviving status-bar leaf is classified as chrome, the whole systemui run drops, app fields and the IME keyboard are handled unchanged, and a synthetic volume-dialog run still survives. A second synthetic case covers the nav-bar leaves (no real nav-bar capture was available on the gesture-nav test device). * fix(replay): surface only meaningful divergence refs, dropping unlabeled structural nodes The get/is/wait divergence uses a full (non-interactive) capture so static-text targets survive, but that also pulls in unlabeled structural containers (ViewGroups/ComposeViews) that carry a ref yet no identity and aren't tappable. On deeply-nested RN trees they consume the SCREEN_REF_CAPTURE_LIMIT budget ahead of the actionable controls (and the app content the excluded status/nav chrome just freed room for). Filter divergence screen.refs to nodes an agent could actually re-target: identifiable (display label/value/non-generic id) or interactive (hittable). * fix(test): run Android status-bar fixture through the real non-raw walk The `simulateNonRawWalk` helper in snapshot-chrome-android-statusbar.test.ts only hand-removed status_bar*/navigation_bar* wrapper nodes, while production (shouldIncludeStructuralAndroidNode in ui-hierarchy.ts) also drops other unlabeled/generic-id structural nodes that aren't hittable and have no hittable descendant. That let the synthetic, non-hittable com.android.systemui:id/home_handle node survive the fixture and get asserted as chrome, when the real walk drops it entirely. Replace the hand simulation with a shared `walkNonRawAndroidFixture` test util that reconstructs the `AndroidUiHierarchy` tree and calls the real `buildUiHierarchySnapshot(tree, undefined, { raw: false })`, so every inclusion/drop decision in the fixture is production's. Update the status-bar leaf assertions to the identifiers that actually survive the walk, and assert `home_handle` is absent (not chrome-classified). Add an Android divergence-route test (`buildReplayFailureDivergence` with `makeAndroidSession`) that feeds the mocked dispatch the real walked tree, covering the target-binding divergence route the previous iOS-only tests missed. Verified the rewritten tests fail when `ANDROID_SYSTEM_CHROME_MARKER_LEAF_IDS` is reverted and pass with it restored. |
||
|
Michał Pierzchała
|
f474f0784e |
feat: unify gesture planning and multi-touch execution (#1212)
* feat: unify gesture planning and multi-touch execution * fix: correct unified gesture helper behavior * refactor: tighten unified gesture architecture * fix: preserve gesture routing contracts * test: account for fresh gesture viewport * refactor: remove retired gesture series * fix: preserve example app navigation targets * test: reconcile unified gestures with helper ownership * docs: update Android helper gesture protocol * fix: refresh Maestro percentage swipe frames * refactor: remove stale Maestro frame cache * fix: harden unified gesture execution * fix: model gesture viewport in providers * refactor: remove legacy gesture paths * fix: remove unused swipe preset parser * refactor: tighten unified gesture boundaries * fix: close gesture review gaps * fix: preserve gesture compatibility contracts * fix: preserve multi-touch recording semantics * fix: refresh Apple runner state after app relaunch * test: lock Apple fling fallback route * fix: close Apple runner review gaps * refactor: tighten unified gesture seams * refactor: consolidate gesture planning policy * fix: preserve swipe response compatibility * fix: keep gesture lab aligned with replay coordinates |
||
|
Michał Pierzchała
|
e2bfed5f9f |
feat(replay): ADR 0012 migration steps 5+6 — resume + --update retirement (#1211)
* feat(replay): ADR 0012 migration steps 5+6 — resume + --update retirement
Step 5 (decision 4, resume): replay --from <n> --plan-digest <sha256>
resumes at a 1-based plan step, skipping 1..n-1 without executing them.
Every divergence report now carries a real resume object (allowed, from,
planDigest, reason?) computed by a preflight that rejects INVALID_ARGS
before any action when: the plan digest no longer matches the current
script (edits/includes/platform-conditioned expansion), --from is out of
range, a skipped step can produce outputEnv values, or the skipped range
or resume target is runtime control flow (retry/runFlow.when — these are
single plan entries, never individually addressable). `test` rejects
--from/--plan-digest both at the CLI-schema layer and at the daemon
dispatch layer (the original command name is only visible before test
rewrites its nested request to `command: 'replay'`).
New modules: src/replay/plan-digest.ts (canonical SHA-256 plan digest)
and src/daemon/handlers/session-replay-resume.ts (preflight + the
report's resume object), kept out of src/replay/ to avoid a
replay<->compat import cycle.
Step 6 (decision 1, retirement): --update/-u no longer rewrites .ad
files. The ADR mandates a no-op, not an error or flag removal: --update
now runs identically to a plain replay and returns the same bounded
suggestions every divergence already carries. Removed: healReplayAction's
retry-and-rewrite arm and its exclusive helpers (collectReplaySelectorCandidates
stays — decision 1's suggestions still use it), the write call from the
runtime loop, and the env/${VAR}-interpolation/compat-flow refusal guards
that existed only to protect that rewrite. writeReplayScript itself keeps
its own round-trip tests but is otherwise unused now; deleted after the
production-exports gate flagged it as dead.
Docs: cli-help.ts workflow topic + --update/--from flag help, AGENTS.md
selector pipeline note, maestro-compat-debt-map.md, website replay-e2e.md
and commands.md updated for the retired rewrite and the new resume loop.
* fix(ci): classify resume flags + provider-scenario resume coverage
The Integration Tests job's architecture-progress gate
(test:integration:progress:check) requires every public CLI flag to be
classified; --from/--plan-digest (replayFrom/replayPlanDigest) were
unclassified. Classify them as device-observable workflow flags and add
real provider-backed coverage to the Android lifecycle scenario: a full
replay diverges on a missing selector, the report's resume object is
asserted (allowed/from/planDigest), and resuming at the next index
replays only the tail. Also refresh the stale replayUpdate reason
("selector-healing replay update" -> the retired no-op).
* fix: bind replay resume digest to execution plan
* test: align replay runtime module topology
* fix: clear replay CI regressions
* docs: clarify replay repair and resume paths
* docs: clarify replay resume step semantics
* docs(replay): note that ${VAR} values stay out of the plan digest (ADR 0012 + workflow help)
Settled decision from the PR #1211 re-review (maintainer-approved): interpolated
${VAR}/--env/AD_VAR_* VALUES are deliberately NOT part of the resume plan digest.
Substitution happens after the digest is computed over the still-unsubstituted
${VAR} text, so re-running the same script with different variable values keeps
the same digest and stays resumable — supplying the right values on resume is the
caller's responsibility. The digest still binds the script/includes, the effective
--platform/--target, and per-action runtime hints + target-v1 identity. Documented
in ADR 0012 decision 4 and the `help workflow` resume topic.
* docs: clarify replay digest interpolation
|
||
|
devin-ai-integration[bot]
|
0a8ea3a57b |
refactor: consolidate architecture ownership and client results (#1210)
* refactor: consolidate architecture ownership and client results Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: keep selector parse chunk grouping current Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: update moved architecture breadcrumbs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: enforce moved selector architecture Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: keep selector guarantee ownership current Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: update selector ownership references Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Michał Pierzchała
|
973df8a8a8 |
feat(replay): ADR 0012 migration step 2 — structured replay divergence report (#1197)
* feat(replay): thread runFlow include provenance through Maestro parsing
ADR 0012 migration step 2. The Maestro parser previously lost per-action
source location in two stages: convertRootCommands stamped every action
produced by inlining a runFlow with the PARENT runFlow: command's own line,
and parseRunFlowFile's callers discarded the included file's line table and
path entirely. This is the live-reproduced bug: "Replay failed at step 5
(__maestroTapOn ...)" with no file or line anywhere in the failure.
ParsedReplayScript gains an optional actionSourcePaths array parallel to
actionLines (undefined means "the top-level replay file"). The Maestro
conversion pipeline now carries {path, line} through runFlow inlining,
including multi-level (include-of-include) nesting, via a transient
SessionAction.replaySource field that convertRootCommands reads back into
the flat arrays and strips before an action leaves the parser.
Regression coverage: a flow including ../launch.yml where a failure inside
the include must report the include's own path+line, plus a nested
include-of-include case.
* feat(replay): structured REPLAY_DIVERGENCE report for step failures
ADR 0012 migration step 2 (report only; kind: "action-failure"). Every
replay/test step failure now returns ok:false with code REPLAY_DIVERGENCE
and a bounded, redacted details.divergence v1 object instead of the
underlying failure's own code:
- step: 1-based plan index + source {path, line}, now accurate through
Maestro runFlow includes (previous commit) and already-correct for .ad.
- cause: the original code/message/hint, preserved verbatim.
- screen: a fresh post-failure interactive snapshot digest, captured and
blessed exactly like settle's stored-tree pattern (setSessionSnapshot +
markSessionSnapshotRefsIssued + session.snapshotGeneration), so refs are
immediately actionable. Capture/sparse failure degrades to
{state:"unavailable", reason, hint} and never masks the original cause.
- suggestions: collectReplaySelectorCandidates + resolveSelectorChain
re-resolution (session-replay-heal.ts), reused READ-ONLY to rank up to 5
candidates by identity-component strength (id > role+label > label >
other) then document order. The same-scrollRegion-as-recorded tier from
decision 1's total order is not evaluated here — it depends on decision
3's recorded target evidence, which this step does not consume.
--update's actual rewrite behavior is unchanged.
- resume: always {allowed:false, reason}, no planDigest/from key until
step 5 lands.
- Bounds: 8/24/64 KiB (digest/default/full) on the serialized
details.divergence object, 256-UTF-8-byte per-field caps, 8/20 screen
refs, 5 suggestions. Overflow writes the fuller detail to a session-scoped
artifact and returns {omittedBytes, artifactPath}, or artifactUnavailable
if the write itself fails — the cause is never dropped either way.
- Success path gains a one-line text summary ("Replayed N steps in X.Xs"),
closing the silent-success gap (text replay previously printed nothing).
Existing replay-failure test fixtures that didn't mock dispatchCommand now
do, since every failure attempts a post-failure snapshot capture.
* feat(cli): render replay divergence as a compact text report
ADR 0012 migration step 2, end-to-end preservation (daemon -> Node client ->
CLI leg). printHumanError now renders a compact divergence summary (step
location, screen availability, ranked suggestions or an omitted-count hint,
overflow artifact pointer) unconditionally — not gated behind --debug, which
previously only showed step+action+selector+a generic hint with no screen
evidence. --json already carried the full structured error via
normalizeError, which was already preserving details.divergence through
stripDiagnosticMeta/redactDiagnosticData (only hint/diagnosticId/logPath/
retriable/supportedOn are stripped there); this commit adds contract tests
proving that survival, plus the throwDaemonError -> AppError leg.
* feat(mcp): treat a replay divergence error as a ref-issuing result
ADR 0012 migration step 2, MCP leg. Previously any command failure (daemon
or otherwise) reached callTool's outer catch, which built a text-only
ToolResult with no structuredContent at all — a divergence's repair data
(screen refs, suggestions, cause) was completely lost over MCP.
The tool executor's own execute() now catches command failures directly, so
it can run the same ref-merge/pin pipeline successful ref-issuing responses
already get: an error's details.divergence.screen (when "available") is
merged and pinned at refsGeneration exactly like a snapshot/find response,
merge-only like every other pin path (an unrelated command error, e.g.
INVALID_ARGS with no divergence, never clears existing pins). The result is
isError:true, structuredContent is the full normalized error, and content
carries the same compact text summary the CLI renders. router.ts's own
catch is now only the fallback for failures outside a resolved command call
(unknown tool name, malformed params) and reuses the same normalize/format
helpers (extracted to mcp/tool-error.ts) rather than duplicating them.
* fix(replay): carry include provenance through retry/runFlow.when wrappers
Review blocker on #1197 (empirically probed): a `runFlow: file:` include
nested under `retry:` or a runtime `runFlow.when: {visible|notVisible}`
conditional broke provenance on both sides. Parse side: the transient
SessionAction.replaySource field was only stripped by the top-level
convertRootCommands path — nested actions under replayControl.actions kept
it, contradicting the field's own "never reaches dispatch" doc comment.
Runtime side: invokeReplayActionBlock invoked every nested action with the
WRAPPING control action's line, so a failure inside such a wrapped include
reported the retry:/runFlow.when: line of the root file — reproducing
exactly the provenance bug this PR fixes, on the single most common real
failure site (the RN suite's launch include is retry-wrapped).
Fix, both sides:
- Parse: stripNestedActionSources removes replaySource from every action a
control-flow wrapper carries and records it as the new parallel
replayControl.actionSources array (undefined entry = "the wrapper's own
file"). Deep leak check test proves no replaySource survives anywhere in
parsed output.
- Runtime: invokeReplayActionBlock/invokeReplayRetryBlock consult
actionSources per nested action; invokeReplayAction threads a sourcePath
(falling back to the wrapper's, then the root file) and attaches the
failing action's resolved {path, line} to the error as a
transport-internal details.replaySource marker — deepest failure wins, an
outer wrapper never overwrites it. Trace events gain sourcePath when it
differs from the root replay file.
Regression tests are the reviewer's probe scenario: a retry-wrapped include
and a runtime-notVisible-wrapped include, each failing INSIDE the include,
asserting divergence.step.source.path/line point into the include (and that
the internal marker is stripped from the flat details). Parser-level tests
assert the strip + actionSources shape for both wrapper kinds.
* fix(replay): single divergence capture; consume nested failure provenance
Review findings on #1197 (needs-work review, findings 1-3 + 5):
- ONE post-failure snapshot now serves both the screen digest and the
suggestion re-resolution. The previous per-purpose double capture had the
second setSessionSnapshot advance the generation past the one the report
had just advertised (screen.refsGeneration=G1, live session at G2), so
MCP pinned the caller's next ref at a stale generation and the daemon
warned "re-run snapshot" — defeating the ADR's ref-issuing contract in
exactly the case a selector suggestion exists. Single capture = one
generation, refs and suggestions minted from the same stored tree, and
one fewer device round trip per failure (heal path drops from 3 captures
to 2, plain failure from 2 to 1).
- Blessing follows the settle/find choke-point sequence with the store
ordered explicitly after markSessionSnapshotRefsIssued, replacing the
ineffective flag flip the review flagged; session-snapshot.ts's
caller-list doc gains the new caller.
- Sparse captures no longer write back (per the selector-capture
reliability contract): a sparse verdict degrades the whole observation to
screen unavailable + no suggestions.
- Capture flavor: interactive-only, except for the non-rect selector reads
(get/is/wait) where heal's suggestion semantics always used a full tree.
- withReplayFailureContext consumes the transport-internal
details.replaySource marker (deepest nested failure wins over the
wrapper's top-level source) and strips it from the flat details; the wire
shaping moved into a pure helper to stay under the complexity gate.
- step.source.line fallback is now ?? 1 (matching the parsers' own
fallbacks) instead of ?? 0, which rendered as "path:0" in the CLI report.
Runtime regression tests cover the reviewer's probe scenario end-to-end
(retry-wrapped and runFlow.when-wrapped includes failing inside the
include) plus a construction-path redaction assertion.
* fix(replay): redact divergence fields before truncation, per the ADR
Review should-fix on #1197: ADR 0012 decision 4 specifies "all rendered
strings and any overflow artifact pass through the central diagnostics
redactor BEFORE truncation"; the implementation truncated at construction
time and relied on normalizeError's later whole-details redaction — i.e.
truncate-then-redact, whose safety depended on the regex redactor's
incidental robustness to secrets split across the cut.
sanitizeReplayDivergenceField (redactDiagnosticData -> truncateUtf8Field ->
marker) is now the sanctioned field sanitizer, used at every divergence
string construction site (cause message/hint, action summary, source path,
screen ref roles/labels, suggestion selectors/labels) and by the minimal
overflow fallback. Tests: a password assignment whose value straddles the
256-byte boundary must yield no secret fragment and a [REDACTED] marker,
plus a no-truncation bearer-token case.
Also picks up the outstanding format-only diff on output.ts (the CI
format:check blocker).
* refactor(replay): one provenance representation through the Maestro pipeline
Maintainer directive on #1197: a workaround needing a paragraph-long
justification comment marks a redesign spot. The named suspect was the
transient SessionAction.replaySource field — attach in runFlow conversion,
strip at two different exits (top-level convertRootCommands, control-flow
stripNestedActionSources), with a doc comment narrating that lifecycle.
The conversion pipeline now returns provenance in a parallel structure
everywhere instead: MaestroConvertedActions { actions, sources } is the
uniform result of convertMaestroCommandWithLine / convertCommandList /
convertRunFlow / convertRepeat / convertRetry. runFlow includes produce
concrete sources; everything else produces undefined ("this file");
control wrappers put nested sources straight into
replayControl.actionSources. SessionAction.replaySource is deleted along
with both attach/strip helpers and their lifecycle docs — one
representation, nothing transient to strip, nothing to narrate.
Comment self-audit across the PR diff: acceptability arguments trimmed to
one-line constraint statements (redaction ordering, single-capture
rationale, --debug gating history, minimal-fallback defensiveness, resume
scope); the arguments live in the PR thread. Behavior unchanged — the
parser/runtime provenance regression tests (retry-wrapped and when-wrapped
includes, deep no-per-action-provenance leak check) pass unmodified in
substance, renamed to describe the carried-structure design.
* fix(replay): categorical fill-text exclusion, repair data on all text surfaces, sanitize/dedupe gaps
Maintainer review blockers on #1197, all three:
1. Fill text never serialized (categorical, not redact-if-secret-shaped).
formatDivergenceActionLabel replaces formatScriptActionSummary (now
deleted — no other consumer remained) for the divergence action field and
the top-level failure message: typing commands (fill/type) drop the typed
value for a `<text>` marker, keeping only the identifying target token
(selector/@ref/point). The flat details.positionals now goes through the
event log's categorical buildDisplayPositionals (`<text:N chars>`).
divergence.cause carries only {code, message, hint} — the fill
verification detail that legitimately holds `expected` is a nested cause
detail and is structurally never serialized, and platform fill failure
messages are static strings. Tests: a sentinel-bearing fill divergence at
every response level asserts the sentinel appears nowhere in the
serialized divergence, flat details, or message; unit matrix for the
label across fill selector/@ref/point and type.
2. Every text surface carries the repair data. The compact divergence
report moved from a CLI-private helper into the shared
formatReplayDivergenceReport (src/replay/divergence.ts) and is now
rendered by all three text surfaces: CLI printHumanError (as before),
MCP text content (tool-error.ts — previously code+hint only), and the
`test` default reporter's failure body (previously message+hint only).
--json and MCP structuredContent were already complete. Tests assert
step location, screen refs line, and ranked suggestions on the MCP text
and test reporter surfaces.
3. Sanitization + dedupe gaps. (a) The capture-failed screen hint
interpolated the raw capture error message — every `unavailable` screen
field now passes through sanitizeReplayDivergenceField (audited all
construction sites; cause/action/source/screen refs/suggestions were
already covered). Test: a secret in the capture error is redacted in
screen.hint. (b) Suggestion dedupe now keys on the node's unique tree
index and keeps the STRONGEST basis per the ADR (previously first-seen
basis won and ref-less nodes could collide/miss). Test: a node reachable
via a label-basis and an id-basis candidate appears once, tagged id.
Also: rebased onto origin/main (ReplaySuiteTestFailed.session fixture
field); the new production-unused-exports gate is clean.
* fix(replay): strip arbitrary cause details from the public divergence error
Maintainer P1 on #1197 (STRIP decision on the escalated scope question):
the flat REPLAY_DIVERGENCE details previously spread the underlying
command error's own details verbatim — and a real fill-verification
failure carries the entered text there (`expected`, unmasked fields, per
the fill-diagnostics contract). Per the ADR ("arbitrary nested cause
details are never serialized"), the transport now drops the cause's
details categorically; only the machine-dispatchable signals
(reason/retriable/supportedOn) survive alongside the transport's own
fields. The documented details-borne meta keys (hint/diagnosticId/
logPath) are hoisted onto the error fields first so repair guidance is
not lost (one pre-existing test updated to the hoisted location).
Privacy regression strengthened to the leak class the selector-miss test
could not see: the failing fill now returns details.expected/actual
carrying the sentinel, and the assertion covers the WHOLE serialized
public error at every response level.
* fix(replay): render repair refs on text surfaces; scrub expanded variables
Review blockers (devin-ai-integration on #1197), both:
1. Text-only repair data was incomplete: formatReplayDivergenceReport
rendered only the ref COUNT for an available screen and dropped the
unavailable hint, so a text-only CLI/MCP/test caller still needed a
follow-up snapshot or a switch to JSON — against the ADR's "no caller
gets a text-only divergence that loses its repair data". The report now
lists a bounded ref/role/label subset (8 lines, matching the digest ref
cap, with a "... N more" remainder) and carries the unavailable-screen
hint. Covered on all three text surfaces (CLI printHumanError, MCP text
content, test default reporter) plus report-level bound/hint tests.
2. Expanded replay variables were not categorically excluded: the generic
redactor cannot know replay-scope values, so `press 'label="${SECRET}"'`
failing with the daemon echoing the RESOLVED selector serialized the
expanded value in divergence.cause and the top-level message (and the
Maestro path bakes expansions into positionals at parse time, reaching
the action label the same way). Every divergence string now passes
through a per-report sanitizer that scrubs all non-builtin replay-scope
values (file env, -e CLI entries, AD_VAR_* shell env, and runtime
outputEnv merges — collected at failure time) to a `<var:NAME>` marker
BEFORE generic redaction/truncation; the top-level message/hint get the
same scrub. Marker replacement, not a drop: the caller still sees which
variable was interpolated. Regression: a selector error echoing an
expanded sentinel (message + hint) asserts the sentinel absent from the
whole public error and `<var:SECRET>` present; plus scrub unit tests
(longest-value-first, non-secret-shaped values).
Fallow follow-ups: the three failure call sites collapsed into one
failStep closure (duplication), divergenceScreenLine split per state
(complexity).
* fix: preserve replay divergence recovery and privacy signals
|
||
|
Michał Pierzchała
|
d968a27dc3 |
fix: honor session metro hints and expo dev-client bundle urls (#1199)
* fix: honor session metro hints and expo dev-client bundle urls - metro reload now resolves against the dev server the session's last metro prepare bound (via a per-session hint file), instead of silently defaulting to localhost:8081 and reloading an unrelated project. Explicit --metro-host/--metro-port/--bundle-url still win. - metro prepare --metro-kind expo (detected or forced) now hints the virtual-metro-entry bundle URL instead of index.bundle, which 404s against Expo dev servers in monorepos (live-verified against react-navigation's example app). Package-manager detection for --install-deps now walks up from --project-root to the nearest lockfile so Yarn/pnpm workspace monorepos don't wrongly fall back to npm and hit EUNSUPPORTEDPROTOCOL on workspace: deps; install failures now hint at --no-install-deps and the detected PM. - open now accepts --metro-host/--metro-port/--bundle-url/--launch-url as session-hint setters (folded into the same runtime object the daemon already persists for open), so a fresh session doesn't need a throwaway reload-first call just to seed hints. Updates help text for metro and open to match. * fix: make the metro-sessions file the single reload hint store Review follow-up (PR #1199): - open's --metro-host/--metro-port/--bundle-url now also record the session's dev-server binding in the metro-sessions file (the one local store metro reload resolves against), so a later plain reload actually reuses what open set. The daemon runtime-hints write stays for device-native dev-server prefs. - The binding now carries bundleUrl (prepare persists the local-flow bundle URL; bridge runtimes are excluded). - clearMetroSessionHints is wired: session close drops the binding (teardown intent — even when the daemon call fails), and a hintless open that creates the session clears a leftover same-name binding. The open result carries sessionReused so the client can tell fresh from reused sessions. Regression test pins prepare -> close -> flagless reload resolving to the default, never the stale port. - Package-manager detection recognizes bun.lock (text lockfile, default since Bun 1.2) and bounds the lockfile walk-up at the nearest .git entry. - Comment audit per maintainer directive: multi-line acceptability arguments trimmed to one-line constraint statements; the store's lifecycle is documented once on MetroSessionHints. * fix: preserve bundle-url mount prefix and broadcast expo reloads over /message Maintainer review follow-ups (PR #1199): - Reload/message endpoint URLs keep the bound bundle URL's mount prefix (e.g. /tenant-42/index.bundle -> /tenant-42/reload) instead of collapsing to the host root. The Expo virtual entry (.expo/.virtual-metro-entry.bundle) is an entry-module path, not a server mount, so it maps to the server-root endpoints (verified live: Expo serves /message at the root). - When the dev server has no HTTP /reload route and answers with the app page (Expo), metro reload now broadcasts {"version":2,"method":"reload"} over the server's /message websocket (the channel dev-server CLIs use for the r key) instead of reporting the app-page 200 as a successful reload. The result carries a transport field (http | message-socket). Live-verified: a flagless metro reload against the running Expo server made the app re-fetch its JS bundle. - Endpoint resolution moved to src/metro/metro-reload-endpoints.ts so the seam tests use is production-imported (keeps the new production-unused-exports ratchet clean). - Docs reconciled: help metro and website commands.md describe the single session binding store, open's hint flags, prefix preservation, and the message-socket fallback. * fix: preserve Expo reload mount prefixes |
||
|
Michał Pierzchała
|
46d2931bf0 | refactor: remove redundant facade exports (#1204) | ||
|
Michał Pierzchała
|
6e21fedc08 | refactor: remove production-unused exports (#1203) | ||
|
Michał Pierzchała
|
9a2277c045 |
fix: alias launch/relaunch to open and suggest canonical commands for unknown names (#1166)
* fix: suggest canonical commands for unknown command names Agents commonly guess command names that don't exist, e.g. relaunch/launch instead of `open <app> --relaunch`, burning turns on Unknown command errors that only say "run --help". Add a curated alias-to-canonical-shape map for the most common guesses (launch/relaunch/start/restart, touch, input/ settext/entertext, screencap/capture, dismiss), backed by a nearest-name edit-distance fallback derived from the live command registry so suggestions can't drift. Also hint that `open` takes the app/bundle id as a positional when an unknown flag looks like a bundle-id guess (e.g. --bundle-id), and apply the same suggestion to `help <unknown>`. Suggestions are display-only; nothing auto-executes and the error code stays INVALID_ARGS. * fix: address review — dead export, case-insensitive suggestions, tighter nearest-name matching - Drop the export on getNearestCommandNames (module-private; only suggestCommandFor uses it) to satisfy the Fallow unused-export gate. - Lowercase the input token before both the curated-map lookup and the nearest-name pass, so RELAUNCH/Relaunch/TAP/Touch get the same hint as their lowercase forms. Added a curated `tap` entry: lowercase `tap` is normalized to press before the unknown-command check, so the entry only catches case variants like TAP. - Tighten the nearest-name fallback: exact prefix matches win outright (`clos` now suggests only `close`, not "one of: close, logs"), otherwise only ties at the minimum edit distance are kept, and 1-2 character tokens never get a suggestion (`ls` no longer suggests `is`). - Share the "open <app> --relaunch" example string between the curated map and the unknown-flag hint, and extend the registry-drift tests to parse each curated example end-to-end (validates open --relaunch as a registered flag) plus assert keyboard dismiss is a real keyboard action. * feat: promote launch and relaunch to true open aliases Follow the tap -> press precedent: `relaunch <app>` now runs `open <app>` with --relaunch injected, and `launch <app>` runs a plain `open <app>` (no forced restart — that would silently destroy app state). Both are normalized in normalizeCommandAlias before parsing, so command identity stays `open` for daemon requests and telemetry, all other args/flags pass through to open's normal validation (URL targets still get the daemon's existing --relaunch guidance), and an explicit --relaunch stays idempotent. Alias matching is now case-insensitive (TAP, RELAUNCH, Launch), so the curated tap suggestion entry is dead and removed along with launch/relaunch; start/restart and the rest of the map stay suggestion-only since start is genuinely ambiguous. |
||
|
Michał Pierzchała
|
c325842f62 |
fix: reap idle daemons and take over stale runner leases (#1169)
* fix: reap idle daemons and take over stale runner leases Each AGENT_DEVICE_STATE_DIR spawns its own daemon that never exits on its own; deleted codex/claude sandboxes leave orphaned daemons accumulating (10+ observed). A stale-but-still-running orphan also keeps holding its iOS runner lease, so a fresh daemon for the same device fails with COMMAND_FAILED "already owned by another agent-device daemon". - Daemon self-reaps after an idle window (default 5 minutes, matching the iOS runner idle-stop default) once it has no open sessions, no in-flight requests, and no active recording. AGENT_DEVICE_DAEMON_IDLE_TIMEOUT_MS overrides the window; 0 disables it. - A runner lease whose owner PID is dead, or whose owner AGENT_DEVICE_STATE_DIR no longer exists, is now reclaimed automatically instead of erroring; a genuinely live owner with an existing state dir still gets the existing rejection + hint. * fix: gate lease takeover on proven-dead owners and fail closed on stat errors Review follow-ups on #1169: - Replace fs.existsSync (which never throws and swallows EACCES/IO errors into "gone") with fs.statSync + error-code inspection: only ENOENT/ ENOTDIR count as proof the owner state dir is gone; any other stat error fails closed and classifies the owner as alive. - Split stale classification by reason (owner-process-dead vs owner-state-dir-gone). Adoption (readStaleRunnerLease -> tryAdoptRunnerSessionFromLease) is now strictly PID-dead-gated: a dir-gone-but-alive owner may still hold a live runner connection, so its lease routes through the force-stop path (kill leased runner processes, rebuild) instead of being silently adopted - no two masters. - Tests: EACCES stat error keeps the busy rejection; dir-gone+PID-alive refuses adoption before probing; dir-gone force-stop asserts a fresh runner launch instead of adopting the old runner pid. * test: make idle reap tests deterministic |
||
|
Szymon Dziedzic
|
cfef0a4bca |
feat: add session event timeline (#1032)
* feat: add session event timeline * fix: support cursor-only event reads * refactor: simplify event log formatting * refactor: trim event log helpers * docs: document session event timeline * refactor: tighten session event log internals * fix: redact event log action positionals by default * fix: align event log after rebase * test: cover events in provider output guard * fix: harden session event privacy * fix: harden event message redaction * fix: harden session event logging --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> |
||
|
Michał Pierzchała
|
2717047b86 |
fix: normalize iOS simulator screenshot density (#1160)
* fix: normalize iOS simulator screenshot density * fix: avoid density metadata after screenshot downscale * fix: satisfy screenshot density CI gates * fix: harden screenshot metadata collection * refactor: centralize screenshot density policy * refactor: reuse screenshot density support check |
||
|
Michał Pierzchała
|
106c238697 | fix: narrow client result contracts (#1155) | ||
|
Michał Pierzchała
|
b0c70ad4e4 | feat: support repack dev server prepare (#1145) | ||
|
Michał Pierzchała
|
7f61df30ae |
feat: add TV remote command (#1147)
* feat: add TV remote command * feat: improve TV remote ergonomics * test: cover tv-remote provider scenario * fix: preserve focused Android TV nodes * docs: tighten PR description guidance * fix: remove d-pad command alias * docs: clarify tv-remote hold syntax * feat: add tv-remote longpress CLI sugar |
||
|
Michał Pierzchała
|
bc7dcc8345 |
fix: keep XCTest tree snapshots on main (#1144)
* fix: keep XCTest tree snapshots on main * fix: address iOS runner snapshot review |
||
|
Michał Pierzchała
|
ef4b66d4dc | test: remove slow-test ratchet pins (#1143) | ||
|
Michał Pierzchała
|
db69124c00 | feat: add capabilities command (#1133) | ||
|
Michał Pierzchała
|
54f6d45b32 | refactor: extract host process primitives (#1134) | ||
|
Michał Pierzchała
|
7475415ac5 |
build: strip runner unit-test blocks from package (#1128)
* build: strip runner unit-test blocks from package * ci: fix package size and fallow checks * fix: resolve packaged recording scripts * fix: keep recording script resolver internal |
||
|
Michał Pierzchała
|
f9721e7e8b |
test: split the Android platform test aggregation and share the scripted adb stub (#1103)
* test: split the Android platform test aggregation and share the scripted adb stub
AGENTS.md names the platform index.test.ts aggregations as offenders to
shrink opportunistically; this splits the 2,735-line Android one along
its (already well-factored) source modules, every test moved verbatim
(92 tests before and after):
- ui-hierarchy.test.ts (22): parseUiHierarchy/androidUiNodes
- app-lifecycle-install.test.ts (13): install/resolve/infer/launch
component parsing
- app-lifecycle-open.test.ts (19): open/close, deep links, launch args,
TV category, fallback resolve-activity
- input-actions.test.ts (11): type/fill/swipe/scroll/rotate
- settings.test.ts (14): appearance/clear-app-state/fingerprint/
permissions
- notifications.test.ts (2), app-parsers.test.ts (1)
- keyboard state/dismiss tests (10) appended to the existing
device-input-state.test.ts
Consistency fix folded in: the file carried a local withMockedAdb fork
because it needs scripted per-subcommand adb responses, which the shared
arg-recorder helper cannot express. The fork now lives in
src/__tests__/test-utils/mocked-binaries.ts as withScriptedAdb next to
withMockedAdb, and hands each call a fresh copy of the shared
ANDROID_EMULATOR fixture.
The copy matters: the Android TV test mutated the callback's device
(device.target = 'tv'), which the old per-call object literal absorbed
silently. With a shared fixture that mutation leaked into the next test
and flipped its launch to LEANBACK. The helper now clones per call and
the TV test builds { ...device, target: 'tv' } instead of mutating.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqeW8sA2ZnvnftdvpCqFMS
* test: serialize the scripted-adb group and repoint its slow-test pins
Review follow-up for the android index.test.ts split: the monolith
implicitly serialized the env-mutating adb-stub tests (PATH,
AGENT_DEVICE_TEST_ARGS_FILE) in one worker, and the split let vitest
run them across parallel files. Make the contract explicit:
- new android-adb vitest project runs the six scripted-adb test files
in a single fork (singleFork), keeping the pre-split execution
semantics; ui-hierarchy and app-parsers stay in the parallel unit
project (pure parsing, no env mutation)
- test/test:unit scripts run both projects
- the five slow-test ratchet pins that referenced index.test.ts keys
now point at the split file names, so the pinned real-time offenders
keep their exemption instead of failing at 2x budget under load; the
reporter's own pinned-key fixture updated to match
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FqeW8sA2ZnvnftdvpCqFMS
* test: use vitest 4 android adb serialization
* docs: update unit project readiness guidance
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Michał Pierzchała
|
2557670193 |
test: slow-test ratchet and speed rules from measured experiments (#1099)
* test: slow-test ratchet, budget-derived emulator poll, speed guidance from experiments Measured (2026-07-04, full unit suite: 340 files / 3,210 tests / 48s wall): wall clock was bounded by the slowest FILE (44.6s android monolith at ~7x file-level parallelism), and the slowest tests were sleeping through real production budgets (10.8s proving 'times out' by waiting the constant out, 8s emulator polls at 1Hz, real retry backoff). Two config experiments rejected with data: --no-isolate exploded the suite to 205s (module state thrashes across files sharing workers) and --pool=threads changed nothing. - scripts/vitest-slow-test-reporter.ts: the slow-test ratchet. Unit budget 2.5s / integration 15s; failure at 2x budget (the band between reports without failing so host-load variance cannot make the gate cry wolf); 36 pinned offenders, exact keys, ratchet-only pin (tracking #1098). - waitForAndroidEmulatorByAvdName: poll cadence derives from the caller's budget (min 1s, floor 50ms, ~timeout/20) — devices.test.ts 25.6s -> 2.8s (9x) in isolation, and short-budget production calls stop sampling at 1Hz against small budgets. - vitest.config: slowTestThreshold 500 for local visibility; reporter wired; isolation/pool decisions documented with the measurements. - docs/agents/testing.md 'Speed rules' + AGENTS.md testing bullet: the three conversion patterns in preference order (budget-derived cadence, budget-wiring assertion, fake clocks), the no-seam constraint, and the file-granularity Amdahl argument that makes the monolith test split a wall-clock fix, not just navigation. * fix: fallow findings on the slow-test gate — import edge, factory reporter, unit tests The string-path reporter wiring read as a dead file (fallow cannot see vitest's reporter loading); the config now imports the factory, making the edge real and type-checked. The class shape tripped the unused-class-members rule (framework callbacks are invisible to reference analysis) — converted to a factory returning the Reporter object, with the classification and rendering logic extracted as pure exported functions. Those functions now carry their own unit tests (budget bands, integration budgets, pin matching, warn-vs-fail rendering), which also grounds the CRAP estimate in real references. Canary re-verified: unpinned 5.2s sleeper fails the run with exit 1; clean runs exit 0. |
||
|
Michał Pierzchała
|
cd0cd16a9e |
build: migrate the library build from rslib to tsdown (Rolldown) (#1087)
* build: migrate the library build from rslib to tsdown (Rolldown) Replace the Rspack-based rslib build with tsdown, the Rolldown-based library bundler from the Vite toolchain family, so bundling, testing (Vitest/Vite), linting (oxlint), and formatting (oxfmt) all run on the same OXC/Rolldown stack. Outcome vs the rslib baseline (size-report): - build time: ~53s -> ~2s - JS raw +16.2 kB (+1.1%), JS gzip +2.7 kB (+0.6%) - the residual gap is OXC vs SWC minifier tightness, not chunking - npm tarball -3.0 kB - CLI --version startup ~3 ms faster; --help within the +/-5 ms measurement noise of interleaved A/B runs Chunk-merging experiments (single shared group, entries-aware groups, small-module groups) all regressed either total size or --help startup (a merged shared chunk adds +140 ms), so the default Rolldown split graph is kept. Custom codeSplitting groups also currently trip a rolldown-plugin-dts bug that re-emits type-only imports as runtime imports. Declarations still bundle per entry via tsgo; dist layout, entry names, and the internal/ worker/daemon entry resolution contract are unchanged. @microsoft/api-extractor was only consumed by rslib dts bundling and is removed together with @rslib/core. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqeW8sA2ZnvnftdvpCqFMS * ci: only cache the pnpm store when setup installs dependencies The layering-guard job uses setup-node-pnpm with install-deps: false, so it never creates a pnpm store. setup-node's post-job cache save then fails with a path validation error whenever the lockfile hash misses the cache - which any lockfile-changing PR does. Gate the cache on install-deps so no-install jobs skip pnpm store caching entirely. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqeW8sA2ZnvnftdvpCqFMS --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Michał Pierzchała
|
c5f7b7baa4 |
refactor: harden exec failure wrapping end to end (#1074)
* refactor: harden exec failure wrapping end to end Follow-up to #1072, closing the structural gaps that let the missing-processExitError bug class exist: - requireExecSuccess(result, message, extra?) in utils/exec.ts: guards an allowFailure result and throws the curated COMMAND_FAILED (flag set via execFailureDetails) itself. Result-side by design — tool providers and executor overrides return results without throwing, so an ExecOptions knob interpreted at spawn time would silently not fire on those paths. ~30 pure guard-and-throw sites across apple/android/web converted; sites with tolerance branches, cleanup, or exit-0 reachability keep their explicit shape. - Source-scan guard (src/__tests__/exec-wrap-guard.test.ts): fails when an AppError('COMMAND_FAILED', ...) details literal rebuilds the stdout/stderr/exitCode trio inline without the helper; intentional holdouts opt out with a documented exec-guard-allow comment. The guard immediately found three wrap sites the July audit missed (runtime-hints run-as probe, device-ready not_ready branch, perf export table) — now converted. - coerceExecResult at the provider boundaries (executor overrides, apple tool provider scope, android adb provider scope): SDK-supplied callbacks cross an unchecked boundary; coercing once there replaces the per-site String(result.stdout ?? '') defensiveness, which is removed. - normalizeError stderr excerpts now strip noise prefixes (adb:/xcrun:/ simctl:/error:) before rendering; skip/strip lists stay in the kernel deliberately (platforms cannot hook normalizeError) with a comment marking the registry escape hatch if they grow. - AppErrorDetails exported and documented in kernel/errors.ts: the magic keys (hint, processExitError, retriable, reason, diagnosticId, logPath, stdout/stderr/exitCode) now carry types and doc comments. - runIosDevicectl gains tolerateOutput; the devicectl uninstall path in app-install.ts reuses it instead of duplicating the wrap + hint logic (its failure hint now falls back to the devicectl default hint). * fix: reconcile exec hardening with the adb failure classifier Main landed the central adb failure classifier (androidAdbResultError + withAdbFailureHintProvider) while this branch was in flight. Resolution: - Android call sites keep main's androidAdbResultError form — it composes execFailureDetails with the classified adb hint, which is strictly richer than a plain requireExecSuccess conversion for adb invocations. requireExecSuccess remains the shape for non-adb tools and apple/web. - Provider result coercion folds into withAdbFailureHintProvider's enrichment pass (exec/pull/install), so the existing WeakSet memo also prevents coercer stacking. - The reverse-remove wrap in adb-executor now uses androidAdbResultError instead of hand-rolling the trio (flagged by the exec-wrap guard). - Guard-test scan split into helpers (fallow cyclomatic threshold) and the perf artifact-tail clone carries fallow-ignore markers on both platforms, matching the pre-existing marker on the apple side. - Two expectations updated for the stderr noise-prefix strip: classifier compose test and perf sampling reason ('device offline', no 'error:'). |
||
|
Michał Pierzchała
|
9aae457533 |
fix(errors): close call-site and consumer gaps around the central error system (#1071)
* fix(errors): close call-site and consumer gaps around the central error system Audit + iOS/Android dogfood findings (see docs/adr/0010-error-system.md): - press/click/fill targets that parse as neither @ref, selector, nor point now fail with INVALID_ARGS grammar guidance (incl. unquoted multi-word selector values) instead of UNKNOWN 'Expected x to be a finite number' - daemon command-input validation throws AppError INVALID_ARGS instead of bare Error surfacing as UNKNOWN - selector-no-match and stale-ref failures carry targeted hints (selectorFailureHint / STALE_REF_HINT) - retriable/supportedOn survive wire rehydration to CLI --json and SDK (previously dropped at throwDaemonError / toDaemonHttpRpcError) - MCP tool errors carry code + hint instead of message-only text - lease busy/capacity use DEVICE_IN_USE (the retriable code) - asAppError(err, fallbackCode) replaces cause-dropping coercions in the Apple runner; new default hints for AMBIGUOUS_MATCH, DEVICE_IN_USE, UNSUPPORTED_PLATFORM, and a distinct UNKNOWN hint - ADR 0010 documents the error-system conventions * fix: format touched files and clear fallow audit gate - privatize SELECTOR_NO_MATCH_HINT / SELECTOR_NOT_UNIQUE_HINT (consumed only via selectorFailureHint in the same module) and integerSchema (only used inside command-input.ts) - dedupe the resolved-node return tail in interaction resolution into describeResolvedNode - extract stringDetail/booleanDetail readers so normalizeError stays under the complexity threshold * fix: reject unquoted trailing text after interaction selectors press/click/longpress positionals like 'press text=Gesture lab' used to silently drop the leftover tokens and act on the truncated selector (text=Gesture), potentially hitting the wrong element. Reject non-empty splitSelectorFromArgs rest with INVALID_ARGS guidance that suggests the merged quoted form (text="Gesture lab"). Fill keeps consuming rest as its text payload; wait/is/replay-heal already handle rest explicitly. |
||
|
Stanisław Chmiela
|
9407839f6e |
feat: tag daemon artifacts with semantic types (#1066)
* feat: tag daemon artifacts with semantic types * Normalize artifact type threading through daemon tracking * Add timeout error to http server artifact test helper * test: restore artifact wait helper behavior * test: fail artifact wait helper on timeout * refactor: model artifactType as optional on wire shapes Applies the design-review tweak: producer-owned APIs (reserveOutput, trackDownloadableArtifact, finalization callbacks) keep the required 'DaemonArtifactType | undefined' form so artifact owners must explicitly decide, while public/wire/result shapes (DaemonArtifact, both artifact inventory entry types) become 'artifactType?:' — missing metadata is valid, JSON drops undefined, and remote/older daemons may omit the field. Construction sites now omit the key for untyped artifacts, and the finalization test asserts key absence (toEqual cannot distinguish absent from explicitly-undefined). --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> |
||
|
Michał Pierzchała
|
1c95bd319b |
fix: accept selector-first is form and print find action confirmations (#1068)
* fix: accept selector-first is form and confirm find actions is <selector> <predicate> failed with a misleading predicate error because every entry point read positionals[0] as the predicate, and the trailing predicate token could not be recovered later: visible/hidden/editable/selected double as selector boolean keys, so greedy selector parsing swallowed it. Normalize both argument orders into the canonical predicate-first shape at the CLI reader and splitIsSelectorArgs, and explain the key/predicate collision in the error hint when nothing parses. find click succeeded silently: the delegated click response (with its 'Tapped @ref (x, y)' message) was discarded, and the find formatter printed fill's raw text field instead of its success message. Carry the success message through find click and print message-first, so find click/fill/focus/ type confirm exactly like their direct counterparts. * style: format cli-grammar test |
||
|
Michał Pierzchała
|
f98111ef7e |
feat(help): document behavioral guarantees in help workflow (#1051) (#1054)
Add a "Guarantees" section to `agent-device help workflow`: statements of fact for agents to reason from instead of probing behavior with trial commands. Covers selector ambiguity (folds/extends the existing #1040 paragraph instead of duplicating it), hittability/targetHittable, open idempotent-foreground vs --relaunch's single simctl launch --terminate-running-process call, close's runner retention policy plus the AGENT_DEVICE_IOS_RUNNER_IDLE_STOP_MS idle-stop window, ref lifetime (cleared by open/relaunch), diff snapshot's comparison baseline, and wait's polling model. Each statement is traceable to source, noted in the PR body. Adds three doc-assertion tests in cli-help.test.ts mirroring the #1040 pattern. |
||
|
Michał Pierzchała
|
5e8a90a924 | refactor: add shared ttl memo reset registry (#1060) |