mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
agent/fix-limrun-dependency-security
1177 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Michał Pierzchała
|
655e8817c2 | fix: secure Limrun dependency packaging | ||
|
devin-ai-integration[bot]
|
61f8696d28 |
test(ci): gate PRs on changed-line coverage (#1418) (#1447)
* test(ci): gate PRs on changed-line coverage (#1418) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor: split coverage-changed model into small helpers for fallow Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(ci): simplify coverage-changed reporting and CLI surface 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> |
||
|
devin-ai-integration[bot]
|
edca35d122 |
chore(deps): Renovate config, packageManager-derived pnpm in CI, repo-wide format (#1444)
* chore(deps): add Renovate config and enforce packageManager pnpm version in CI Refs #1422 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: bump pnpm to 11.17.0 and format the whole repo with oxfmt format/format:check drop their hand-maintained path list: oxfmt already skips node_modules and honors .gitignore, so the only exclusion list is .oxfmtrc.json ignorePatterns. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mutation): accept either quote style in the affected-lane path filter Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore(deps): keep fixture-app runtime deps as individual Renovate PRs 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> |
||
|
devin-ai-integration[bot]
|
e89ef9e69b |
test: gate public command surface against commands.md reference (#1446)
* test: gate public command surface against commands.md reference Enumerate PUBLIC_COMMANDS against website/docs/docs/commands.md in both directions with a waivered unit-lane gate, and document the drift found on main (doctor, react-native). Refs #1420 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: recognize tilde-fenced code blocks in command-doc gate Refs #1420 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
|
c984bd5902 |
fix(ci): upload hidden diagnostics (#1443)
* fix(ci): upload hidden diagnostics * fix(ci): capture all hidden diagnostics |
||
|
devin-ai-integration[bot]
|
006c4cadc9 |
test: nightly parser fuzz lane — parser input fails as typed AppErrors, never hangs (#1414) (#1438)
* test: nightly parser fuzz lane with typed-AppError invariant (#1414) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): run envelope, artifact promotion, and harness self-check tests (#1414) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): shared scheduled-lane envelope on every terminal path, watchdog after ready (#1414) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): envelope for malformed options; add scheduled-lane health consumer (#1414, #1430) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(lanes): actions:read scope, terminal error envelope, first-due grace (#1414, #1430) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(lanes): anchor first-run grace to schedule registration, use exec helper in tests (#1414, #1430) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(lanes): portable POSIX pickaxe pattern for schedule registration (#1414, #1430) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(fuzz): fast-check generators over the shared hazard list, drop the bundled lane-health work (#1414) - Strip scripts/scheduled-lane/* and scheduled-lane-health.yml: that watcher is #1430's own deliverable and collides with PR #1439's implementation of the same lane. What this lane owes (a per-run envelope) moves into scripts/fuzz/envelope.ts. - Rebase onto #1437 and rebuild the generator layer on fast-check: cases come from arbitraries sharing SELECTOR_VALUE_HAZARDS with the property suite, and counterexamples are shrunk, so a failure names a minimal input plus fast-check's seed/path instead of a 20k-char random string. - Route harness.test.ts into the serialized subprocess-stub project. - Drop the AGENT_DEVICE_FUZZ_STARTUP_DELAY_MS test seam: the ready handshake is now proven by a case budget far below real worker startup. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): replay the regression corpus through the worker watchdog (#1414) A promoted hang case used to wedge the unit job until the CI timeout, because corpus replay called checkCase in-process. It now goes through the same worker-backed watchdog the nightly lane uses, so such a case fails against a 5s per-case budget; the file moves to the serialized subprocess-stub project with the rest of the worker-driven fuzz tests. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): let the watchdog outlive vitest's default case timeout (#1414) A wedged parser was surfacing as a bare 'Test timed out in 5000ms' instead of the named hang: failure that says which input wedged, because the file's vitest timeout was shorter than the watchdog budget times the number of replayed cases. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): complete drift provenance in the lane envelope (#1414) configHash now covers every input that decides what a seed generates (generate.ts and the shared property arbitraries, not just the arbitraries/targets/invariant), and tool records fast-check's installed version. A generation-loop edit or a fast-check upgrade previously changed the case set while the envelope looked unchanged. A test recomputes the hash with each input omitted so a future omission fails. 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> |
||
|
devin-ai-integration[bot]
|
e545544dfa |
test(daemon): seeded concurrency torture lane for session/lease/lock invariants (#1439)
* test(daemon): seeded concurrency torture lane for session/lease/lock invariants Refs #1416 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): drive torture lane through real lock plan + review fixes - derive each op's lock plan from production resolveRequestExecutionLockKeys via a fake device-inventory provider, so reverting the router's same-device serialization trips the overlap invariant (verified) - assert exact replay: full scheduler trace, terminal outcome, contention - assert real same-device lock contention in the sweep + a forced 2-client case - split harness into bindings/invariants/envelope modules (all <500 LOC) - emit #1430 scheduled-lane envelope (schema/SHA/hash/seed range/duration/result) and upload it from the nightly workflow Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): pass claim data as plain view accessors (fallow) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): gate lock plan on shouldLockSessionExecution; sweep replay + forced-device contention; whole-lane envelope Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): add real-scope runLocked serialization guard; whole-lane envelope duration Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): allow seed 0 replay; add seed-0 regression (TORTURE_SEED must accept 0) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * obs(#1430): add scheduled-lane freshness/cadence health watcher Discovers schedule: workflows from .github/workflows/, reads recent scheduled runs via the GitHub API, and opens/pings a tracking issue when a lane misses or fails two consecutive cadences. Pure model unit-tested and gated on PRs; API I/O + issue open/ping run nightly. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): give scheduled-lane watcher a two-cadence newborn grace Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): anchor lane grace on schedule-introduction, not workflow age Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): derive schedule-activation semantically via git, through runCmdSync Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): add merge-commit regression pinning first-parent + committer time Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): pin scheduled-lane-health issue-write route via stubbed fetch Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): unbundle #1430 watcher; make torture lane nightly-only Strip the scheduled-lane-health watcher (scripts + workflow + PR gate) — it is #1430's deliverable and collides with PR #1438's workflow of the same filename; keep only this lane's #1430 envelope writer. Move the torture lane under test/integration/nightly/ so it is out of the test:integration:node glob, and run it via an explicit, disclosed PR step plus the nightly sweep. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: fix stale torture-lane paths after nightly/ move Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): adopt shared lane-envelope for torture lane Rebase onto main (post-#1441) and replace the lane-local LaneEnvelope dialect with the shared scripts/lib/lane-envelope.ts builder, so the #1430 health watcher parses one schema: commitSha->commit, sourceHash-> configHash, seedRange/runs moved into the typed data payload, and the sweep encoded as seed "<start>-<end>". 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> |
||
|
devin-ai-integration[bot]
|
8cce0ef6b8 |
test: ratchet mutation score over enumerated decision kernels (#1441)
* test: ratchet mutation score over enumerated decision kernels Adds a Stryker (vitest runner) mutation lane scoped to the decision kernels, a per-module baseline with tool/config provenance, and a ratchet that only lets scores rise. Non-gating until two consecutive stable weekly sweeps. Refs #1415 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: declare the mutation test-scope seam for production-export analysis Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: own kernel tests in the mutation registry and ship the #1430 lane envelope - restore bench:help-conformance, broken by a formatting-path edit - kernel test files select their module on PRs (registry `tests` + workflow paths), asserted to reach the kernel through the import graph - every mutation run writes the standard scheduled-lane artifact envelope - move src/utils/__tests__/errors.test.ts beside its source per the mirror rule Refs #1415, #1430 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: derive kernel test ownership and land the scheduled-lane health monitor Ownership of a kernel's tests is now computed from the static import graph (scripts/mutation/ownership.ts) instead of a hand-listed set, so a test that reaches a kernel indirectly -- src/__tests__/daemon-error.test.ts through src/daemon.ts -- selects that kernel on a PR. The PR lane triggers on every src test and shards the derived modules, keeping wall clock at one module. The lane envelope (#1430) is now written on every exit path with the stage it reached, so a crash before any mutant runs is distinguishable from a lane that never ran. Adds the derived cadence monitor (scripts/lane-health, daily workflow): scheduled lanes are enumerated from .github/workflows/ and reported dark, failing, or never-run against their own cron cadence. * fix: merge only Stryker reports from a shard directory The shard artifacts now carry the lane envelope beside mutation.json, and the merge globbed every .json under the download path, so the ratchet job fed the envelope to the report parser and died after the mutants had already run. * fix(mutation): fail on incomplete shard sets and envelope pre-run failures Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(mutation): downgrade a passing envelope when a later lane step fails Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mutation): shard by registry, defer the PR lane, drop the bundled watcher Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: describe registry sharding and the deferred PR mutation lane Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(mutation): make the pre-graduation tooling exception select real mutants Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mutation): give the worktree fixture commits their own identity 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> |
||
|
devin-ai-integration[bot]
|
d747ef6230 |
test: frozen replay-compat corpus with expected verdicts (#1417) (#1436)
* test: frozen replay-compat corpus with expected verdicts (#1417) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: pin replay-compat corpus bytes to released blobs and assert via parseReplayInput Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: lock replay-compat provenance kind by corpus area and verify it in CI Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: describe corpus provenance-kind lock and CI job Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: prune replay-compat corpus to minimal witnesses per shipped form Reviewer feedback on #1436: the mechanism earns its place, the dataset did not. Drop the 30 corpus entries whose bytes repeat a syntactic form or a migration refusal another entry already witnesses (platform twins and adjacent-release re-recordings), leaving 22 deliberate entries; make note required and state per entry which form or refusal it is the sole witness of. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: address corpus review nits (typed coverage list, cap rationale, derived-citation note) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: split corpus rule — form from the release, verdict from today's parser Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: format corpus README emphasis markers 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> |
||
|
Andrew Barnes
|
4c02b6ad2b |
fix(cli): reject excess positionals (#1433)
* fix(cli): reject excess positionals * fix: preserve get ref labels in arity checks --------- Co-authored-by: Andrew Barnes <169967362+Bortlesboat@users.noreply.github.com> Co-authored-by: Michał Pierzchała <thymikee@gmail.com> |
||
|
Michał Pierzchała
|
fcaa6c995c |
refactor(contracts): declare shared vocabulary below its consumers, ratchet what remains (#1435)
* refactor(contracts): declare the public API vocabulary below its consumers
The layering gate's largest remaining cluster was 28 type-only inversions from a
single edge: `commands/` declaring itself in terms of `client/client-types.ts`.
R2 forbids the reverse import, so a shape both surfaces need has to sit below
both. The command/device vocabulary — connection config, the device and session
views, and every per-command Options/Result — now lives in
`contracts/client-api.ts`; `client/client-types.ts` keeps the `AgentDeviceClient`
facade and re-exports the rest through one wildcard.
R6 total: 42 -> 18. No new inversion in any pair.
The published surface is unchanged, and that is verified rather than asserted:
the built `index.d.ts` exports the same 216 type names as main, byte-identical.
Eight shapes deliberately did NOT move, because each is stated in terms of a
HIGHER-ranked zone: `ScrollOptions` (ScrollInputDirection, commands/), the four
navigation Options plus `AgentDeviceCommandClient` (navigation-projection,
commands/), and the two Metro result aliases (metro/). Declaring those in
contracts/ would trade 28 commands->client edges for contracts->commands and
contracts->metro ones — the foundation depending on the layers above it, worse in
kind even though fewer in number. This is measured, not assumed: moving the whole
file to contracts/ first took the gate from 42 to 48, which is how the floor was
found.
Two keystone moves made the other 84 movable:
- `RemoteConnectionProfileFields` joined its sibling `CloudProviderProfileFields`
in contracts/remote-config-fields.ts. It was the root of the base chain
(AgentDeviceClientConfig -> AgentDeviceRequestOverrides ->
DeviceCommandBaseOptions -> every per-command Options), so one rank-4
declaration was pinning ~80 shapes up with it.
- `DaemonBatchStep` moved to contracts/batch-step.ts. Its `runtime` field was
written `DaemonRequest['runtime']`, dragging the whole daemon request type in to
say `SessionRuntimeHints` — the same type, three zones lower.
`CompanionTunnelScope`/`MetroBridgeScope` also moved to contracts/, since the
vocabulary needs the scope shape and it sat next to client-local env-var names.
Six pass-through re-exports in client-types.ts are suppressed per-name with the
reason inline: they exist only to publish contracts/kernel types through the
package entrypoint wildcard, every internal consumer imports them from the
declaring module, so "no consumer" is correct and not actionable — deleting them
would remove names from the public types.
`pnpm check` green, 4488 unit tests. Findings doc records the sequencing for the
last 5: the upstream declarations have to come down before the shapes that need
them can.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur
* docs: drop the graph viewer, keep the query that replaces it
The rendered dependency-graph viewer is not being merged (PR #1409 closed). It cost
~2200 lines plus a Fallow exemption for a 920-line canvas renderer, and nobody —
human or agent — reached a conclusion from the picture. Every finding in this
document came from short queries against the gate's own model.
This file pointed at the `claude/depgraph-viewer` branch for the tooling, which
would have dangled once that branch is deleted. Replaced with the thing that was
actually load-bearing: a throwaway probe script, inlined, that re-derives the
numbers from `scripts/layering/model.ts` and nothing else. Verified verbatim — it
reproduces TYPE_INVERSION_BASELINE exactly, which is also the check that tells you
whether either side has gone stale.
Two numbers in the summary table were stale, describing an intermediate state
rather than what shipped: R6 said "35 across 4" (actually 18 across 5 after the
vocabulary move) and ranked coverage said "729 of 894" (actually 888 of 901). Both
corrected, along with the file/edge counts in the header.
Also notes the deduplication detail that makes the query agree with the gate: each
file pair counts once, so a raw edge count reads higher.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur
* refactor(contracts): move the four keystones that pinned the rest of the inversions
R6 type-only spine inversions: 18 -> 7, and every one of the 7 that remains is a
deliberate architectural position rather than a misplaced declaration.
Four keystones moved to contracts/, each of which was pinning a much larger set:
- `CommandFlags` (was core/dispatch-context.ts). One rank-2 declaration holding the
daemon's request type and every recorded action above it. Its last non-contracts
dependency was `DaemonBatchStep`, already moved in 3fdbfe0.
- `SessionAction` (was daemon/types.ts). replay/ (6 modules) and compat/maestro/
read and write session scripts; declaring the shape inside the daemon made both
depend on the server to describe a file format neither asks it to produce. The
daemon still owns the recording — only the shape moved.
- `TargetAnnotationV1` shape (was replay/target-identity.ts). ADR 0012 target
evidence, written by 8 daemon modules and read by commands/; the parsing and
classification logic stays in replay/.
- `ScrollInputDirection` and the Metro prepare/reload result payloads, which
unblocked `ScrollOptions` and `MetroPrepareResult`/`MetroReloadResult`.
`DaemonRequest` also split into the three shapes it had been conflating: the
kernel WIRE shape (`flags?: Record<string, unknown>`, because a process boundary
cannot enforce a vocabulary), the new `contracts/command-request.ts`
`CommandRequest` (wire shape with flags typed — what a command surface needs), and
the daemon's own refinement (+ `internal?: DaemonRequestInternal`, carrying
SessionState callbacks and the admitted lease). core/command-descriptor/ had been
importing the third to read `command`, `positionals` and `flags`.
Two things deliberately NOT moved, because moving them would add coupling rather
than remove it, and the baseline now argues both:
- `DaemonCommandDescriptor`/`DaemonCommandRoute` — the route type is
`keyof typeof DAEMON_ROUTE_HANDLERS`, derived from what the server implements.
Moving it down means re-declaring route names in contracts plus a gate to prove
the handler map still covers them. ADR 0003/0008 own that boundary.
- `AgentDeviceClient` — used as an opaque handle by 4 files. The facade is built
from commands/'s own NAVIGATION_COMMAND_PROJECTIONS, so this is a genuine
zone-level cycle; breaking it is a design call about where that registry belongs.
R5 is zero here: nothing imports the client at runtime, only its type.
Also records the largest structural finding, which R6 does not measure: cycles by
edge kind are 1 (value only), 87 (value + type-only), 1 (value + dynamic), 213
(all). At runtime the graph is a clean DAG; the 87-file type-level cluster means
no one of those files' types can be read in isolation. Hubs are
runtime-contract.ts, commands/runtime-types.ts, backend.ts,
commands/runtime-common.ts. Not attempted here — it is a different and much larger
change.
`pnpm check` green, 4488 unit tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur
* feat(layering): ratchet type-cycle growth (R9), and rule out a narrower client port
R9: the largest strongly-connected component over value + type-only edges may not
grow. R4 keeps the VALUE graph acyclic, so every cycle counted here is created by
type-only imports - free at runtime, invisible to R5/R6, and the largest single
obstacle to reading a subsystem in isolation: inside a component of 102 files, no
file has a self-contained slice.
Baseline set to 102, which is what THIS branch achieves - main carries 107 and the
boundary moves here bring it to 102. An earlier revision baselined 87, measured
against an older main; after rebasing onto
|
||
|
devin-ai-integration[bot]
|
8670a88afa |
test: property-based testing foundation (fast-check) over parse/print and geometry kernels (#1437)
* test: property-based testing foundation over parse/print and geometry kernels (#1413) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: derive selector and replay command domains from their owning registries 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
|
e8b779cb32 |
fix(daemon): keep close-time script-save failures from leaking the session/device claim (#1392)
* fix(daemon): keep close-time script-save failures from leaking the session/device claim A close-time script write (implicit from `open --save-script`, or this close's own `--save-script`) that refuses to publish (e.g. a no-clobber target-exists AppError) threw uncaught out of `handleCloseCommand`, skipping lease release, device-claim release, and `sessionStore.delete` entirely — while the `close` action had already been recorded with no rollback. Live-repro'd over the real CLI against an Android emulator: this single gap explained both symptoms split out of #1384 into #1391 — a lingering `DEVICE_IN_USE` claim after a failed `close`, and a published `.ad` rewritten with duplicated trailing `close` lines when the same close was retried (each attempt re-recorded a `close` action on top of the one never rolled back from the prior failure). Catch the write failure, roll back the just-recorded `close` action (mirroring the existing repair-armed commit-failure pattern), and let teardown (lease release, device-claim clear, session delete) complete regardless — exactly as an ordinary platform-close failure already doesn't block them. The failure is still surfaced to the caller, but after teardown, with a corrected hint: retrying the same close is no longer meaningful since the session is now gone. Fixes #1391 * refactor(daemon): shrink handleCloseCommand/runSessionCloseTeardown under fallow's complexity gate CI's fallow code-quality check flagged handleCloseCommand (126 lines, 19 cyclomatic / 16 cognitive) and runSessionCloseTeardown (73 lines) as exceeding the large-function/high-complexity thresholds after the prior commit's fix. Extract runCloseTeardownAndRelease (teardown + lease release + claim clear + delete + ordered error surfacing) and buildCloseSuccessResponse (final response shaping) out of handleCloseCommand, and finalizeOrdinaryCloseScript out of runSessionCloseTeardown. No behavior change — same control flow, split into named, independently-readable steps; fallow now reports 0 complexity findings for this diff. * fix(daemon): preserve the write error's structured details in the close-time save failure Review feedback on #1392 (thymikee): toOrdinaryCloseSaveScriptFailure rebuilt the AppError from only the original message, dropping its machine-readable details.reason ("script_target_exists"), details.path, and cause. A caller dispatching on those fields (or reading the CLI's --json error.details) lost them even though the underlying write failure carried them. Preserve the original error's details/cause, overriding only the close-specific hint and retriable:false. Extends the #1391 regression test to assert the routed close response still carries reason/path. * refactor(daemon): drop the vestigial close-time rollback, add router-level #1391 coverage Review feedback on #1392 (thymikee), P2 items: - The close-time save-script failure's session.actions rollback (finalizeOrdinaryCloseScript) was left over from an earlier design where a failed save could keep the session alive for retry. It never does now — runCloseTeardownAndRelease always tears the session down regardless of the outcome — so there is no surviving session for a later write to duplicate the close action on. Drop the rollback; the durable events.ndjson entry (which the rollback never touched anyway) and the in-memory action now agree, both accurately recording that the close happened. - Add a request-router-level regression (request-router-typed-error.test.ts, alongside the existing repair-close BLOCKER 2 test it mirrors) proving the normalized JSON error shape a real client sees: top-level retriable:false, details.reason/path preserved, and the session torn down — not just that handleCloseCommand throws the right AppError when called directly. * test(daemon): assert the durable close event survives a failed close-time save Review feedback on #1392 (thymikee), final P2 item: the previous commit removed the actions rollback because there's no surviving session to duplicate the close action on, but nothing actually asserted the durable events.ndjson action.recorded:close event stays put. Flush and read it back so a future rollback or event-order change can't silently recreate the in-memory/durable mismatch the removed rollback used to paper over asymmetrically. * test(daemon): assert the retained session's in-memory close action, not just the durable event Review feedback on #1392 (thymikee): the durable-event assertion alone doesn't catch a reintroduced session.actions.length = actionsBeforeClose rollback, because that event is queued (and durable) before the write even attempts — a regression there would leave the assertion passing while silently reintroducing the in-memory/durable mismatch. Retain the session object past handleCloseCommand (store.delete only drops the map entry, not the object a local variable still points at) and assert its actions array contains exactly one close entry, matching the durable event count. Verified by temporarily reintroducing the old rollback locally: this assertion fails (0 !== 1) where the prior durable-only check did not, then reverted. * refactor(daemon): model repair close retry as receipt * refactor(daemon): merge blockingError to state, not explain, the save-script exclusion Following up on the comment-trimming pass already on this branch: the device-claim condition (!platformCloseError && !cleanupAggregate) and the two-line throw sequence right below it both needed a paragraph explaining why saveScriptError is excluded from one but not the other. Merge platformCloseError and cleanupAggregate into a single named blockingError — its name now states the exclusion the comment used to argue for, and the throw sequence collapses from two ifs to one. Trimmed the remaining long docblocks in this file the same way: state what's non-obvious in 1-3 lines instead of re-deriving it in prose. * refactor(daemon): clarify close script finalization |
||
|
Michał Pierzchała
|
a7989769f9 |
Update README.md to simplify CLI usage instructions
Removed unnecessary commands from README instructions. |
||
|
Michał Pierzchała
|
6dae319c90 | fix: update vulnerable dependencies (#1434) | ||
|
Michał Pierzchała
|
3ccd97a43d |
docs: bless the works-today iOS SpringBoard/widget workflow (#1296 PR A) (#1395)
* docs: bless the works-today iOS SpringBoard/widget workflow (#1296 PR A) Live probe on iOS 26.2/Xcode 26.2 proved open com.apple.springboard already binds a driveable SpringBoard session with zero code changes: the full widget add/edit/remove flow is selector-driven from a fresh snapshot, aside from two documented coordinate fallbacks. Add a help ios-system-ui topic (and cross-links from physical-device/workflow, the skill router, and docs/commands.md) so agents can use it today, ahead of the --ui-target contract and gallery-capture fix landing. * docs: scope the SpringBoard claim to verified iOS simulator support #1296 explicitly leaves physical-iPhone SpringBoard unverified; the only evidence so far is an iPhone simulator run, whose private-AX fallback is simulator-only. Applied to both the CLI help topic and website/docs/docs/commands.md, with a link to the tracking issue. Dropped the SkillGym case from this PR per thymikee: most of that harness is being removed in #1411, so it's not worth iterating on here. * fix(test): keep help ios-system-ui out of the 30-line first-screen budget #1404 added a benchmark gate requiring every Agent Workflows pointer to stay within the first 30 lines of bare `agent-device help` output. Adding ios-system-ui to that list pushed help macos to line 31. Drop the AGENT_WORKFLOWS entry; the topic stays fully reachable via its cross-references from help physical-device, help workflow's Escalate section, the agent-device skill router, and website/docs/docs/commands.md. |
||
|
Michał Pierzchała
|
7b1fc3bdd3 |
docs: architecture principles mapped to enforcing gates and open ledgers (#1428)
* docs: map architecture principles to their enforcing gates and ledgers in CONTEXT.md Writes down the rule → gate → ledger mapping (dependency rule → layering R1-R3/spine + R6 ratchet; acyclicity → R4 + depgraph tolerated-cycle report; policy×detail → the two registries + leak guard + transport seam; information hiding → R7; boundaries-are-earned with the #1409/platform-descriptor evidence; component metrics as observatory data per #1423) so contributors and agents get the why behind the guards they hit, in one place. * review: scope principle claims to actual enforcement; mark depgraph as proposed P1: the section referenced scripts/depgraph/ as present and #1410 as kept — that PR is open and this tree has no depgraph directory. Both the acyclicity and component-metrics bullets now name it as proposed (#1410), not landed. P2: enforcement claims scoped to the concrete guards: publicPlatformString gates one serialized-output DTO class, runnerProvider is one adopted seam, R7 covers SessionState only, and boundaries-are-earned plus stable-interface testing are labeled norms (gated only at the test-only-DI-seam slice). The intro now says explicitly that bullets distinguish gate, ledger, and norm. |
||
|
Michał Pierzchała
|
2d1d70613f |
feat(bench): renderer-pinned samples, topic-coverage gate, error-recovery quizzes; trim skillgym to agentic checks (#1411)
* feat(bench): renderer-pinned samples, topic-coverage gate, error quizzes; trim skillgym to agentic checks The help conformance bench's quoted CLI output is now sourced from scripts/help-conformance-sample-outputs.mjs, and every sample is rebuilt through the real production renderers (settle output formatters, printHumanError, formatSnapshotText, refMutationAdmissionResponse) by scripts/__tests__/help-conformance-sample-outputs.test.ts — a rendering or message change fails deterministically instead of leaving the bench grading against output the CLI no longer prints. This retires the fabricated recoverable-failure envelope (production never throws a textual settle timeout; that case is replaced by a real DEVICE_IN_USE recovery quiz). Bench cases move to scripts/help-conformance-cases.mjs and are enumerated against the help-topic registry: helpTopicIds() is exported from cli-help, and scripts/__tests__/help-conformance-topic-coverage.test.ts fails when a help topic has neither a bench case nor an explicit waiver. New case families: error-envelope recovery quizzes (device-in-use, stale pinned ref, ambiguous find match, app-not-installed) pinned to real error text, topic coverage for tv/web/react-native/debugging/workflow, and a metamorphic twin of the settled-diff quiz. The skillgym smoke suite shrinks from 119 cases to the 5 that measure what only an agentic runner can show: skill routing plus output interpretation with a proven local CLI help probe (local-cli-help-policy). Its embedded samples now import the same pinned constants, replacing hand-transcribed output that had already drifted from the renderer. Knowledge checks belong to the bench; live fixture behavior belongs to the iOS simulator e2e suite. * review: drive error samples through the real producers; enforce local-help on the routing smoke The DEVICE_IN_USE, AMBIGUOUS_MATCH, and APP_NOT_INSTALLED parity tests no longer hand-author the producer message before rendering: each drives the actual producer — buildDeviceInUseBySessionError (extracted in session-open.ts and called by the handler), buildAmbiguousMatchError (now exported from find.ts), and buildAppNotInstalledError (extracted in app-resolution.ts and thrown by the resolver). Because each factory is exported from its producer file and called by the production path, dropping the production call would make it test-only and fail check:production-exports — the wiring is gate-enforced, not conventional. open-and-snapshot now sets requireLocalCliHelp and allowOnlyLocalCliHelpCommands, so the 'skill plus local help' claim is observed rather than assumed; without them the case can pass on model prior alone. |
||
|
Michał Pierzchała
|
f19864e486 |
feat(scripts): dependency-graph report over the layering gate's model (#1410)
* feat(scripts): dependency-graph report over the layering gate's model
Reports what the layering gate deliberately does not enforce, as JSON plus a short
summary. No renderer: the productive artifact is the JSON.
pnpm depgraph # -> .tmp/depgraph/graph.json + summary
pnpm depgraph:test
Dependency graph: 898 files, 4627 edges, 25 zones
value-import cycles (R4): 0
type-only/dynamic cycles (not gate-rejected): 8
spine back-edges (R5): 0
type-only spine inversions (R6): 42
transitively redundant value edges: 1338
The two numbers worth having are the ones CI cannot give you. Transitively
redundant value edges — where the target is still reachable at distance >= 2, so
the direct import changes nothing about what the module can see — need a real
reachability pass, not a grep. And cycle detection over type-only and dynamic
edges covers the loops R4 excludes by design. Both are candidate lists, never work
lists; at ~1300 the redundancy set is a place to look.
It reuses scripts/layering/model.ts, the same module check.ts uses in CI, so the
file set, zone partition, edge kinds and cycle definition are the enforced ones. A
second extractor would describe a graph nobody gates. Consequence worth having:
its R6 count reproduces TYPE_INVERSION_BASELINE, so a mismatch means one of the two
is stale.
This is the analysis half of a viewer that was built and dropped. The render cost
~2200 lines and needed a Fallow exemption for a 920-line canvas file, and nobody
read it. Everything here clears the repo's bar with NO exemption — scripts/depgraph
is deliberately absent from ignorePatterns, unlike scripts/layering, scripts/perf
and scripts/maestro-conformance.
Getting there meant fixing rather than suppressing: extracted `valueSuccessors`
(the value-edge adjacency was built identically in two places — a real clone),
split `buildGraph` into four named aggregation steps, split
`reachableBeyondDirectEdge` out of `markRedundantEdges`, extracted
`compareZoneEdges`/`crossedZonePair`, extracted `edgeKindCode`/`edgeFlags` from a
nested ternary scoring CRAP 42, and deleted `fileGroup` plus the `group` node field
once the cluster layout went.
Two additive exports on scripts/layering/model.ts: `zoneRank` and `targetDagZone`
(previously module-private). The gate's behaviour is unchanged.
`pnpm check` green, 4488 unit tests, 5 model tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur
* ci(layering): assert the depgraph report reproduces the gate's baseline
The report reads the same model as the gate, so its inversion count must equal
TYPE_INVERSION_BASELINE. That agreement was previously a nice property nobody
checked; the Layering Guard job now runs scripts/depgraph/model.test.ts, so the
two cannot be green independently. Verified by bumping a baseline entry by one and
confirming the job fails with a message naming the fix.
The count feeding the check is computed by `typeInversionsByPair`, which applies
the gate's rule — once per FILE pair, over the raw resolved edges — rather than
reading the collapsed edge list. That matters: `collapseEdges` keeps one edge per
pair with the strongest kind winning, and `dynamic` outranks `type`, so a module
imported both lazily and for its types would collapse to `dynamic` and drop out of
the count. No such pair exists today (measured: 0 of 42 inverting pairs), but a
number wired into a CI equality check must not be able to drift for a reason
unrelated to layering.
Stated honestly in the README and the test: this is a cross-check of the report's
extraction and the baseline against the real tree, not two independent algorithms.
The gate remains the authority — if they disagree, the baseline or the tree is
wrong, never the test.
TYPE_INVERSION_BASELINE is now exported for this purpose.
Not done here, deliberately: the ~1338 transitively redundant value edges are a
candidate for a loose growth-only ratchet later. They are a candidate list, not a
work list, and a hard count would be noise.
`pnpm check` green, 4488 unit tests, 6 depgraph model tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur
* fix(depgraph): make the source reviewable, and stop overclaiming removability
Four review findings, all of them real.
P1 - the implementation was binary. scripts/depgraph/model.ts contained two raw NUL
bytes used as map-key delimiters, so Git classified a ~346-line file as binary and
hid its entire diff behind `- -`. Replaced with a unicode escape: identical at
runtime, textual on disk. I had seen the symptom repeatedly - every grep on that
file printed "binary file matches" - and worked around it with python instead of
asking why, which is how it survived to review.
Guarded repo-wide rather than for this one file: a new test asserts no tracked .ts
under src/ or scripts/ contains a raw NUL, verified by reintroducing one and
watching it fail. Nothing else would catch a recurrence, and the failure mode is
silent - the code works, the review does not.
P1 - "transitively redundant" claimed removability it cannot support. Module
reachability does not carry bindings: if `a` imports `{ c }` while `b` only
re-exports it as `{ c as b }`, the path a -> b -> c exists and deleting a -> c still
breaks `a`. The fixture in model.test.ts is exactly that shape and its comment said
"removable". Reachability also says nothing about when a module's side effects run.
Renamed throughout to what it measures - `transitivelyReachable`,
`markTransitivelyReachableEdges`, and a summary line reading "value edges whose
target is also reachable at distance >= 2 (reachability only - not a removability
claim)". The caveats and the counterexample are now stated in the marker function,
the fixture comment and the README, and symbol-level analysis is named as what
deciding any individual edge would actually require.
P2 - build.ts had no coverage. Every test exercised model.ts, so the CLI could break
its output path, wire shape or summary silently. Added three subprocess tests:
default path plus summary-agrees-with-payload, `--out` honoured and valid JSON
written, and a trailing `--out` falling back rather than crashing (pinned so it is a
decision, not an accident). `pnpm depgraph:test` now runs inside `check:tooling`, so
`pnpm check` covers it.
P2 - README was wrong three ways: it queried `.tmp/depgraph/index.json` after the
output moved to `graph.json` (the documented command failed as written), it derived
inversions from collapsed `zoneEdges`, which can undercount, and it claimed both
that the report runs in CI and that nothing here runs in CI. The query now reads
`typeInversions` and was run verbatim; the CI sentence names exactly which single
test runs and states that nothing else gates a merge.
pnpm check green, 4488 unit tests, 10 depgraph 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
|
5ef52ad901 |
fix(replay): bounded retry on pre-dispatch capture racing app launch (#1385) (#1386)
* fix(replay): bounded retry on pre-dispatch capture racing app launch (#1385) A step right after `open --relaunch` (e.g. a step-2 press) can have its pre-dispatch target-verification capture land while the app is still launching/mounting, producing a transient capture-failed/sparse-snapshot verdict that isn't a real divergence and fails the step closed before it ever dispatches. captureDivergenceObservation now takes an opt-in retryLaunchRace flag that bounds-retries that specific capture (fixed backoff, 12s deadline), mirroring wait's keep-polling landmark semantics (#1349) on this pre-dispatch path. Only verifyReplayActionTarget's gate opts in; the post-failure diagnostic capture and post-resolution guard-mismatch capture stay single-shot since they follow an already-real failure. * fix(replay): gate #1385 launch-race retry on content-quality vs mechanism failure Address review feedback on #1386: the bounded retry was retrying every thrown capture-failed, including permanent mechanism failures (e.g. a missing helper artifact) that a retry can never fix, at the cost of the full backoff budget before the identical divergence. Gate the retry on the same signal Android's helper capture path already emits for this exact distinction: retriable:true on a content-poor rejection (rejectAndroidHelperContentUnavailable), unset on a permanent one (androidSnapshotHelperUnavailableError). The non-throwing sparse-snapshot verdict still always retries. Mirrors #1381's isUnreadableCaptureContentError taxonomy for the wait keep-poll loop. Also update the two #1385 tests to use a retriable-tagged error and add a case proving a permanent failure fails on the first attempt. * fix(replay): narrow #1385 retry to a real content-quality taxonomy, anchor the deadline Address second review pass on #1386: P1 - `retriable === true` was too broad: Android's adb layer (adb-executor.ts) marks genuine transport mechanism failures retriable too (connection_dropped, device_offline, server_version_mismatch - an unchanged retry of the SAME adb command can succeed there), so those consumed the launch-race retry budget despite being exactly the mechanism failures the ADR says must fail fast. Replace the check with the narrow discriminator: the thrown error's androidSnapshotHelperFailureReason must be one of the three literal content verdicts rejectAndroidHelperContentUnavailable attaches (empty-helper-output, system-window-only, content-poor-app-window) - never the free-form message androidSnapshotHelperCaptureError sets for a crashed/timed-out helper, and never present at all for a permanently missing helper artifact. Added a regression test for an adb mechanism failure marked retriable at the transport level, proving it still fails on the first attempt. P2 - the 12s deadline started AFTER the first (unbounded) capture attempt, so the effective wall-clock cost was "first capture + 12s of retries", undocumented and untested as such. Anchor the deadline before the first attempt instead, and rewrite the comments/ADR to state precisely what is and is not bounded: the delay list caps attempt count, the deadline caps sleep time from entry, neither caps an individual capture's own duration. Added a fake-timer test proving the deadline (not just the delay array's length) is what stops retries when captures themselves consume real time. * test(replay): add targeted iOS sparse-snapshot retry regressions Address the reviewer's second finding on #1386: the Android capture-failed retry path had recover/exhaustion regressions, but the iOS sparse-snapshot verdict branch (isSparseSnapshotQualityVerdict, always-retryable, no thrown error involved) had no test of its own - only incidental coverage by way of the Android-shaped tests. Add the mirror pair: a sparse verdict that recovers within the bounded retry and dispatches, and one that never recovers and fails closed as identity-unverifiable once the retry is exhausted. * fix(replay): re-consume shared #1381 predicate after rebase reintroduced local one The branch's rebase onto main re-resolved the #1381 conflict without carrying forward the earlier fix, reintroducing a local ANDROID_CAPTURE_CONTENT_QUALITY_REASONS/isCaptureContentQualityFailure duplicate of the shared isUnreadableCaptureContentError predicate (src/snapshot/snapshot-quality.ts, landed by #1381). Two parallel copies of the same taxonomy can silently drift. Drop the local duplicate and its Android-platform-type import; consume the shared predicate directly, matching what ADR 0012 already documents. |
||
|
Michał Pierzchała
|
9b48e11672 |
fix: support xctrace-only iOS physical devices (#1375)
* fix: support xctrace-only iOS devices * fix: route xctest runner through usbmux * refactor: simplify ios physical device transport * refactor: remove unused runner export * fix: route physical iOS operations through device control * fix: preserve macOS runner screenshots * test: cover xctest capability parity |
||
|
Michał Pierzchała
|
e9eeb45b8e |
fix: select app-bearing simulator for replay (#1387)
* fix(replay): bind app-aware simulator before replay * fix(replay): preserve deep-link target resolution * fix(replay): infer iOS app selection without platform * fix(replay): preserve authored Android selection * fix(replay): ignore deep links during device binding * test(replay): cover authored Android platform * fix(replay): defer binding after deep links * fix(replay): retain platform before deep links * fix(open): preserve platformless device resolution |
||
|
Michał Pierzchała
|
ab913c9720 |
feat: strengthen agent help benchmarks (#1404)
* feat: strengthen agent help benchmarks * fix: harden help benchmark review findings * fix: close help benchmark validation bypasses * fix: make selector scoring quote-insensitive |
||
|
Michał Pierzchała
|
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
|
256887f194 |
feat(apple): injectable Apple runner transport seam for provider interactors (#1389)
* feat(apple): injectable runner transport seam for provider interactors (#1297) createAppleInteractor now accepts an optional AppleRunnerProvider (or bare command executor). When injected, every runner-command method runs inside withAppleRunnerProvider scope, so the shared selector/tap/fill/scroll/ snapshot stack rides the provider transport instead of local XCTest — mirroring createAndroidInteractor's AndroidAdbProvider parameter. Methods backed by local Apple tooling (simctl/devicectl: open, openDevice, close, screenshot, clipboard, setSetting) fail fast with UNSUPPORTED_OPERATION in provider mode instead of silently running local tooling against a remote device; provider sessions compose their own implementations on top. Local behavior is unchanged: without the new parameter the factory returns the same interactor as before, and daemon-owned sessions keep resolving the local XCTest runtime. * fix(daemon): request-boundary provider runner scope + per-request interactor context Review findings on #1389 (P1/P2): P1 — daemon routes that issue Apple runner commands outside interactor methods (keyboard, native alert, point read, iOS sequence chunks) escaped to the local XCTest runtime for provider devices. ProviderDeviceRuntime now exposes getAppleRunnerProvider; createProviderDeviceRuntimeRequestProviders composes it into an appleRunnerProvider request resolver and the daemon runtime wires it, so the existing request-boundary scope covers those routes with the provider transport. P2 — per-request RunnerContext was discarded for provider devices: getInteractor threads it through getProviderDeviceInteractor into ProviderDeviceRuntime.getInteractor, so runtimes composing the shared Apple interactor keep requestId (cancellation/accounting), appBundleId, and log paths per request. Lease-route commands (lease_allocate/heartbeat/release, artifacts) now skip sessionless provider-device resolution: they manage lease lifecycle, not a device session, and resolving a default device there spuriously triggered local device discovery before any lease existed. Integration coverage: keyboardDismiss reaches the provider transport via the request scope; every runner call in a request carries that request's id. * test(daemon): assert direct-route runner calls keep the request id through the provider scope Re-review follow-up on #1389: the keyboard-dismiss provider-scope test now sends a requestId and asserts the recorded runner call carries it, proving the request-boundary appleRunnerProvider scope preserves per-request context for direct daemon routes (not just deviceId matching). * fix(daemon): revert-sensitive transport tests, route-derived lease skip, guarded provider-scope resolve Review round 3 on #1389: 1. The integration acceptance test passed with the interactor transport param removed — the request-boundary scope was routing for it. Tests now run in two worlds: the shared-stack and per-request-id tests use a world WITHOUT getAppleRunnerProvider (the interactor param is the only seam; verified failing when the param is dropped), while the direct-route test keeps the request scope it pins. 2. skipSessionlessProviderDevice for lease-route commands is now derived from daemon.route === 'lease' in shouldSkipSessionlessProviderDevice instead of hand-spread across four descriptors, with a registry-driven invariant test enumerating the route. 3. resolveScopedProviderDevice catches resolveTargetDevice failures and returns undefined: provider-scope plumbing failing to find a device means 'no provider scope', never a failed request. Also collapses the duplicated provider-scope Proxy from android.ts and apple/interactor.ts into core/interactor-scope.ts, and restates the transport param's role (local-tooling partition + out-of-daemon scoping) in its doc comment. * fix(core): move the provider-scope proxy below the ranked spine Layering Guard (R5 zero-back-edges) rejected platforms/apple/interactor.ts value-importing core/interactor-scope.ts: platforms (rank 1) may not import core (rank 2). The helper needs nothing from core — generic withMethodScope in utils (unranked, already imported by both zones) replaces it. The prior local layering pass was a false green: the guard enumerates tracked files and the new helper was untracked when the gate ran. |
||
|
Michał Pierzchała
|
152894cce2 |
docs(adr): rules-first ADR restructure + ADR 0017 proposal (unified event journal) (#1399)
* docs(adr): rules-first restructure of 0012/0014/0016, drop completed migration logs ADR 0012 alone was 42% of the ADR corpus by bytes; consulting it cost ~28k tokens of mostly process history. Restructure per the new shape convention (added to the ADR README): Status + a normative 'Rules at a glance' first so a reader can stop after ~50 lines, rationale and refuted alternatives kept below the fold, and completed migration plans/landing tables deleted — git history is the archive. - 0012: delete migration plan/progress; fix the Status section that still claimed #1235 unimplemented against its own landing table; demote the 2026-07-10 evidence audit to the end (still cited by the decisions). - 0014: same; the accepted Android blocking-dialog-recovery evidence gap and its covering fixture tests move into Status so the waiver survives. - 0016: verified implemented; rules summary added (nothing was history). No rule's meaning changed; edits are reorganization plus stale-status fixes. * docs(adr): propose ADR 0017 — unified request event journal Apply ADR 0008's registry thesis to events. Inventory (2026-07-24) found four parallel event vocabularies — ~155 stringly-typed diagnostics phases, the session events.ndjson, the progress wire stream, and the replay timing trace (one of its two writers unredacted) — with consumers coupled to emit sites by string: agent-cost counts runner round-trips by matching two phase names. Proposal: an EVENT_CATALOG in contracts making every kind a typed, trait-carrying declaration; the diagnostics scope becomes the single journal append point; every consumer becomes an explicitly registered sink; all existing file/wire formats stay byte-compatible behind golden fixtures. Explicitly rejects pub-sub and event sourcing. Status: Proposed — not indexed in the ADR README until accepted. * docs(adr): revise ADR 0017 per architecture review Address all five review findings and adopt both requested judgments: - P1 out-of-request events: finalizeRepairTeardown records a synthesized close during idle-reap/daemon-shutdown with no live request; a request-scoped-only journal would silently drop it. Added an explicit session-scoped teardown scope model (fatal-scope precedent) and rejected the ambient-fallback alternative. - P1 redaction vs byte-compat: progress stays unredacted on its own channel; the replay-trace unredacted->redacted change is now a declared, intentional compatibility change with its own fixture update, not smuggled under a byte-compat claim. - P1 per-attempt trace routing: sinks with dynamic destinations read scope-bound routing context (logPath-rebind precedent); drop-when-unbound semantics; sink ordering/isolation/flush contract made normative. - P2 progress typing: progress streaming removed from the journal entirely - it is a transport-owned output port (ordering, disconnect-as-cancellation, closed typed union); mirror emits noted as the future opt-in shape. - P2 completeness check: orphan detection is now a static source scan in the layering-lint style; runtime unit-suite observation explicitly rejected. Also per review: catalog keys are internal identities; sinks map to legacy wire discriminators, which are never automatically canonical. Migration plan reduced to 4 steps. * docs(adr): ADR 0017 — fork, never rebind, for per-attempt trace routing Review found a blocking concurrency flaw in the revised routing design: sharded test attempts run concurrently (Promise.allSettled in runReplayTestShards) under one inherited AsyncLocalStorage request scope, so mutable scope rebinding would let one attempt overwrite or clear another's replay-timing destination after an await — cross-writing or dropping events. Replace rebinding with a journal fork primitive: journal.fork(bindings, fn) runs fn in a new ALS scope object sharing the parent's buffer/phaseCounts/ envelope/sinks but carrying frozen routing bindings. Each attempt wraps its work (including nested replay dispatch) in a fork binding its own trace path; the binding dies with the fork, so no clearing step exists to race. Existing updateDiagnosticsScope rebinds stay confined to sequential request setup, pre-fan-out. Validation gains a concurrent-shard regression proving each replay-timing.ndjson contains only its own attempt's events. * docs(adr): ADR 0017 — scope identity on the envelope for future exporter sinks Reserve the one shape decision an OTel-style exporter would otherwise force a retrofit for: every scope (request, teardown, fork) carries scopeId, forks record parentScopeId, both ride the event envelope. Forks already form a tree, so an exporter sink can emit parent-child spans from envelope fields alone. Cross-process correlation stays requestId; a traceparent-style meta field is additive under ADR 0006 and deferred. No exporter in this ADR. * docs(adr): address re-review — renumber to 0018, full fork isolation, 0016 record-as 1. Renumber the proposal 0017 -> 0018: main now carries accepted ADR 0017 (parameterized recorded inputs, #1369); branch rebased onto it. 2. Fork contract strengthened: forks clone EVERY mutable scope field (envelope, logPath, routing bindings) and own their event buffer; only the sink list and the request-global phaseCounts tally are shared. Verified in code: nested dispatch creates child execution scopes (request-router.ts:257) whose updateDiagnosticsScope rebinds session/logPath mid-flight, so a shared mutable envelope would cross-route debug/session-log events between concurrent shards even with frozen trace bindings. updateDiagnosticsScope now specified as mutating only the innermost scope. The regression now covers all three routed outputs (replay-timing, per-request diagnostics ndjson, events.ndjson). 3. 0016 rules summary updated for shipped #1348: sensitive fills use fill --record-as <VAR> (ADR 0017); unparameterized fill/type stays literal (body sections already updated by #1369's merge). * docs(adr): ADR 0018 — name the usage sink as first consumer, privacy by construction The motivating consumer is opt-in usage analytics over agent behavior: command frequencies, typed failure codes, and outcome sequences that trip agents (consecutive snapshots, screenshot-after-snapshot). Decision 4 pins its discipline now, before any exporter exists: an allowlist-by-construction UsageRecord schema whose every field draws from a registry-enumerated vocabulary (command names, ADR 0010 error codes, flag names, durations, hashed session + sequence number) — positionals, selectors, labels, fill text, and error messages are unrepresentable by type, not redacted. Anti-pattern detection is downstream analysis over the stream, never emission-side logic; the sink itself is a follow-up after migration step 3. Adds the matching invariant and a schema gate to validation. |
||
|
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 |
||
|
Andrew Barnes
|
14be01b781 |
fix(replay): preserve cwd scope for opened sessions (#1401)
Co-authored-by: Bortlesboat <169967362+Bortlesboat@users.noreply.github.com> |
||
|
Michał Pierzchała
|
1a76344685 |
docs: restructure AGENTS.md and CONTEXT.md for progressive disclosure (#1402)
* docs: restructure AGENTS.md and CONTEXT.md for progressive disclosure
Apply the Claude 5 context-engineering guidance to the repo's agent docs:
keep the always-loaded file to gotchas and invariants, and move situational
guidance one hop away behind a routing table.
AGENTS.md 315 -> 229 lines. Cut generic agent-behavior boilerplate, three-way
duplication (Common Mistakes restated Hard Rules; Finding Source Owners
restated the registry section), and facts visible from the repo itself.
Kept verbatim: the expensive-lessons principles, enforcement gates, Hard
Rules, and environment traps.
Split out docs/agents/{cli-flags,pull-requests,device-verification}.md and
folded the Testing Matrix into docs/agents/testing.md, reframed around
pnpm check:affected so the prose stops duplicating the selector.
CONTEXT.md keeps all 50 terms, now grouped under a section index so a task
loads one section instead of the whole glossary.
* fix(check-affected): move the selector-owning sentinel to the Testing Matrix
The Testing Matrix moved from AGENTS.md to docs/agents/testing.md, but the
affected-check selector still treated only AGENTS.md as selector-owning. A
later matrix edit would have been classified as inert docs and skipped the
fail-open, so the selector could keep deriving gates from a spec that had
changed underneath it.
Move the sentinel with the prose, as a named SELECTOR_OWNING_DOCS set so the
next move is one line, and fix the two in-code comments plus the testing.md
paragraph that still pointed at the AGENTS.md matrix.
* docs: restore two rules dropped by the AGENTS.md split
Review caught two repo-specific rules that did not survive the move. Both are
prose without any backticked identifier, so the identifier-diff used to verify
the split could not see them.
- "Test through public interfaces; do not add unrelated production exports
solely to enable tests" returns next to the behavioral-tests rule in
docs/agents/testing.md, with the reason it exists.
- The guidance-ownership rule (decide whether new guidance/schema/metadata
belongs to the command surface, CLI grammar, CLI help, MCP projection, or
daemon runtime) returns to the always-loaded Docs & skills section, since it
governs all command-surface work and not just the flag case.
Also point the ADR routing row at docs/adr/README.md, which is already the
"read when you touch…" index, rather than at the bare directory.
|
||
|
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
|
11e0a1f187 |
feat: add WebView accessibility lab (#1397)
* feat: add WebView accessibility lab * refactor: tighten iOS snapshot presentation rules * fix: preserve semantic WebView containers |
||
|
Michał Pierzchała
|
5507a08b9c |
feat: parameterize sensitive recorded inputs (#1369)
* feat: parameterize recorded inputs * fix: harden parameterized replay recording * fix: sanitize parameterized fill echoes * fix: scrub embedded parameterized fill echoes * fix: make recorded fill scrubbing idempotent * fix: replay parameterized coordinate fills * test: align parameterized publication landmark |
||
|
Michał Pierzchała
|
5a50cfb892 |
fix(daemon): keep an active replay session's daemon alive over the CLI path (#1390)
* fix(daemon): keep an active replay session's daemon alive over the CLI path A `replay <script>.ad` with no terminal `close` reports its session as still active per ADR 0016's consumption contract, but the real CLI client tears down the daemon that ran it (and its owned ephemeral state dir) regardless — the request's own success response gets overwritten seconds later by an empty `session list`. This happens whenever the client started the daemon itself, independent of whether the state dir was randomly generated or passed explicitly via --state-dir/AGENT_DEVICE_STATE_DIR, matching #1384's live repro. Add `sessionActive` to `ReplayCommandResult`, computed from whether the session survives in the daemon's own store (never by re-parsing the script), and gate the client's one-shot teardown on it — mirroring the existing ADR-0012 repair-divergence keep-alive. A kept-alive owned daemon now also attaches a --state-dir address hint to the response so the caller can reach it. `test` is unaffected: its own per-file runner already closes each session before the suite summary is built. Fixes #1384 * fix(daemon): fix CI formatting, address #1390 review feedback - oxfmt --check flagged the new test file; reformatted (CI fix). - Address hint now names --session <name> too, using the response's session verbatim (already the fully-qualified cwd-scoped store key) — a bare --session default only resolves by coincidence from the same cwd, per resolveEffectiveSessionName's explicit-flag bypass. - Add a test closing the loop: a follow-up sendToDaemon using the hinted --state-dir/--session reaches the same kept-alive daemon without spawning a new one. - Pin that a completed (non-diverging) --save-script repair also keeps its daemon alive via the same guard, since its terminal source close is always skipped (ADR 0012 Fix 3) — document the resulting deferred heal-commit timing in ADR 0012. * test(daemon): reduce complexity of new active-session tests for CI gate Fallow's audit gate (new-only findings) flagged the two new active- session tests for exceeding the CRAP threshold. Extract the shared fixture wiring into replayLeavingSessionActive/parseAddressHint helpers (also cutting duplication between the two tests), and drop repeated optional chaining on response.data in favor of a single narrowing assert.ok(data) — same assertions, lower branch count. * fix(daemon): add sessionActive to MCP schema, real-producer tests, ADR fix Addresses the second review pass on #1390: - MCP replay output schema (src/mcp/command-output-schemas.ts) omitted the new required sessionActive field entirely; add it. - ADR 0016 still claimed "the absence of close changes ... nor the success response shape", contradicting the new required field this PR adds. Amend it to document the sessionActive contract and why the real CLI/IPC client needs it (issue #1384). - All prior lifecycle tests exercised sessionActive only through a fake HTTP response in the client-layer tests, so deleting either real producer line (session-replay-runtime.ts, session-replay- maestro-response.ts) would not have failed anything. Add tests against the real runReplayScriptFile producer (native .ad close-less -> true, terminal close -> false, Maestro close-less -> true) and strengthen the provider-scenario (real daemon route) test with the same assertion. Verified each new test fails when its corresponding producer line is reverted, then restored. Live-validated the fix on real backends (booted iOS 16 simulator and a running Android Pixel 9 Pro XL emulator), replaying issue #1384's exact repro end to end: the owning daemon and its session both survive a close-less replay and remain fully addressable via the hinted --state-dir/--session on both platforms. That validation surfaced a separate, pre-existing bug -- `session list` (no explicit --session) omits cwd-scoped sessions opened via a replay's internal `open` dispatch, because session-open.ts's resolveImplicitSessionScope(req) sees a different req than the top-level replay request and leaves session.sessionScope unset -- filed as #1394, out of scope here since it is a sessionScope-propagation gap unrelated to the client-side teardown this PR fixes; the session itself is never actually lost. * fix(daemon): hint --session at explicit state dirs too, shell-quote the hint Addresses the third review pass on #1390 (P2 x2): - withActiveSessionAddressHint (renamed from ...IfOwned) no longer suppresses the active-session hint entirely for an explicit --state-dir/AGENT_DEVICE_STATE_DIR caller. The session name is cwd-qualified and, per #1394, `session list` can't rediscover it either, so --session is now hinted regardless of ownedStateDir; --state-dir is only included when the state dir is the client's own randomly-generated one the caller has no other way to learn. - attachActiveSessionAddressHint now shell-quotes (shellQuoteIfNeeded, the same helper session-recovery-hints.ts/request-lock-policy.ts already use) both the state dir and session name, so the hint stays literally copy-pasteable even if either contains spaces or shell metacharacters. Added tests pinning both the unsafe-value quoting and that quoting was actually exercised (not just coincidentally unchanged) -- verified they fail against a raw-interpolation reversion, then restored the fix. |
||
|
Michał Pierzchała
|
9b610fbd1e |
feat(replay): recorded landmark identity for wait, is coverage — read-only step identity (#1349) (#1381)
* refactor(replay): extract shared target-evidence tree helpers into src/replay Move buildIndexMap/buildAncestryChain/filterIdentitySet out of the daemon's session-target-evidence into the shared replay zone so the commands runtime (wait's polling loop, #1349) can consume them without importing the daemon; press-retarget drops its private buildIndexMap duplicate. * feat(replay): recorded landmark identity verification for wait, get-pattern coverage for is (#1349) - New CommandDescriptor trait targetIdentityVerification pins the evidence-carrying command set and routes wait to a post-resolution phase so an annotated wait never enters the generic pre-dispatch verification (an absent landmark is its expected starting condition). - wait <selector> records landmark-mode target-v1 evidence (existence self-check; identity-empty matches record no annotation) and, on replay, keeps polling until a selector match carries the recorded identity; a deadline with only impostor matches fails closed as an identity-mismatch REPLAY_DIVERGENCE, a recorded-unverifiable annotation refuses before polling, and a plain timeout stays an action-failure divergence. - is (except exists) joins the get pattern: evidence at record time, generic pre-dispatch verification, and the post-resolution guard threaded through dispatch; direct-iOS fast paths for wait/is are gated during recording and guarded replays. - Read-only find stays intentionally unannotated (fuzzy-locator resolution has no selector-chain identity token), proven by test. * feat(publication): destination guard requires verified recorded landmark identity (#1349) A qualifying ADR 0016 guard is now a selector wait whose target-v1 annotation is verified; identity-less or unverifiable guards are refused with a recovery hint. Adds the reshuffled-screen false-pass regression: record -> publish -> replay against a same-label/different-ancestry tree diverges as identity-mismatch (matchCount >= 1 proving the selector alone would have false-passed). * refactor(replay): dedupe post-dispatch identity-mismatch shaping, trim evidence-writer complexity Shared buildPostDispatchIdentityMismatchResponse behind the guard and wait-landmark conversions; extracted payload-ceiling helpers from computeTargetEvidence; identity-refusal conversion split out of resolveReplayStepResponse. Docs: ADR 0012 decision 3 amendment (#1349), ADR 0016 guard strengthening, help workflow/save-script text. * refactor(replay): make landmark evidence's record-time verification explicit, trim ADR-restating docs The landmark-mode self-check was provably a tautology (the winner is a member of its own identity set whenever the parent walk is intact), so a membership scan defended only by a comment is replaced with the explicit decision: broken walk fails closed, landmark is verified by construction, action mode keeps decision 3's step-5 self-check. Doc comments that re-argued the ADR amendment now state behavior and point to it. * chore: untrack multitouch-helper build artifacts, ignore its build/dist dirs Generated Android helper output swept into the earlier refactor commit by accident; analogous snapshot-helper/ime-helper build dirs were already ignored. * fix(interaction): wait polls ride out content-unreadable captures (live-validated on Android) Live ADR 0016 validation on a Pixel emulator showed a destination-guard wait replayed immediately after a navigation press deterministically dies: the first poll's capture lands mid-transition and the Android helper's 'insufficient foreground app content' verdict threw out of the polling loop. iOS already yields the same state as a sparse verdict with no matches, so the loop kept polling there — this makes wait semantics platform-consistent. A content-verdict capture failure (isUnreadableCaptureContentError) now counts as a no-match poll for selector and text waits; a wait whose screen never became readable rethrows the last capture verdict at the deadline, so persistent breakage keeps its diagnosis. Other capture failures still throw immediately. * fix(snapshot): narrow unreadable-capture classification to enumerated content verdicts Android stamps androidSnapshotHelperFailureReason on mechanism failures too (helper timeouts, adb failures, missing helper artifact — free-form reason strings), so matching any string made waits poll those to their deadline instead of failing immediately. The predicate now matches only the enumerated content-recovery reasons, and AndroidHelperContentRecoveryDecision derives its reason union from the same list so a new content verdict cannot miss the predicate. Adds the realistic wrapped mechanism-error regression the synthetic test missed. * test(interaction): make the wait mechanism-failure regressions revert-sensitive Assert exactly one capture attempt: the broad any-string classifier would poll the repeated fixture error to the fake-clock deadline and rethrow the same message, passing the message-only assertion. Verified the mechanism test fails against the broadened classifier and passes against the narrowed one. |
||
|
jibin jose
|
968db8b26f |
fix(daemon): explicit abort message on uncommitted repair close (#1383)
* fix(daemon): explicit abort message on uncommitted repair close Fixes #1380 * style: run pnpm format * fix: loud abort for close --save-script on uncommitted repair * fix: address static check failures from review |
||
|
Michał Pierzchała
|
a0ab735ffd |
fix(interaction): direction-named, selector-first off-screen recovery hints (#1366) (#1374)
* fix(interaction): direction-named, selector-first off-screen recovery hints (#1366) An agent that hits a scrollable form section gets correctly rejected for targeting an off-screen element, but the recovery hint didn't name a concrete next move. Its "obvious" retry — re-issue the same @ref after scrolling — is exactly what both live guards reject: the off-screen guard still sees it off-screen, and the scroll expired the ref frame (ADR 0014), so the @ref is refused as stale. Two bootstrap-bench runs burned all 60 turns on a single checkout-form screen this way (arm-independent). Make the rejection self-sufficient for recovery instead of relaxing either guard: - Off-screen selector/ref rejection now names the exact scroll direction (computed from the target rect vs its effective viewport) and steers the retry to a *selector*, which re-resolves against a fresh snapshot and bypasses the ref-frame admission guard entirely. The direction is also surfaced as a machine-readable `scrollDirection` detail. - `scroll`'s unknown-direction error now carries a grammar hint, since the transcripts show agents mis-shaping it as `scroll @ref down` — scroll takes a direction and no target. New geometry helper `classifyOffscreenScrollDirection` maps a rect + viewport to the reveal direction (largest-overshoot axis), following the same convention the CLI off-screen summary already uses. Both guards keep their exact rejection semantics and messages; only the hints and one added detail field change. * fix(interaction): derive scroll direction from the boundary that rejected (#1366 review) Address P1: the direction classifier used a rect-vs-single-viewport test, so it only fired when the whole rect was fully past an effective-viewport edge. But `isNodeVisibleOnScreen` rejects on two boundaries — and the second one (tap-point CENTER outside the ROOT viewport while the rect still overlaps its container) is exactly the off-screen-drawer / edge-straddling case #1366 is about. Those got the generic hint and no scrollDirection, leaving the loop unresolved. `classifyOffscreenScrollDirection` now takes (node, nodes) and mirrors both rejection boundaries: full separation from the effective viewport, then a center pushed outside the root viewport. Direction is taken from whichever boundary failed, using the same unrounded center as the tap-point rule, so a rejected target always yields a direction. Regressions: partial clip whose center is past the viewport edge (both the pure classifier and the live selector-press rejection path), and a child inside an off-screen scrollable ancestor (closed drawer). * style: oxfmt the windowRoot test helper (#1366 CI) * fix(interaction): bounded off-screen recovery hint — a large scroll overshoots (#1366 review) Live evidence exposed that the hint over-promised: it advertised `scroll <direction>` -> retry-selector as the terminating recovery, but on the motivating checkout form a plain `scroll` (iOS fling momentum) repeatedly overshoots the narrow pressable band and the loop never terminates — even bounded `scroll --pixels 200` oscillates up/down. A momentum-free `gesture pan` in small steps lands the target (verified: 3–4 attempts -> Tapped "Cash"). Revise the hint to prescribe BOUNDED movement in the named direction, retrying the same selector after each step, and to name the overshoot failure mode and the reliable `gesture pan` fallback. Direction + selector-first steering are unchanged. `scrollRevealClause` drops the "to bring it on-screen" over-promise. Tests assert the bounded-recovery guidance (small steps / gesture pan) on both a selector and a ref off-screen path so it can't silently regress. |
||
|
Michał Pierzchała
|
d45190613c | fix: report bundle sizes with two decimal MB precision (#1382) | ||
|
Szymon Dziedzic
|
3faeb97855 |
feat(events): enrich session event details (#1379)
* feat(events): enrich session event details * fix(events): harden session event projections * fix(events): sanitize provider-derived metadata * fix(events): close remaining projection gaps |
||
|
Michał Pierzchała
|
67d5d21cf8 | fix(android): preserve fixed siblings after snapshot filtering (#1378) | ||
|
Michał Pierzchała
|
5cea83d994 |
fix: select iOS simulator by installed app (#1376)
* fix: select iOS simulator by installed app * fix: preserve app-aware open selection |
||
|
Michał Pierzchała
|
00ef28734f |
refactor(commands): derive pass-through command bindings with a generic BoundOf helper (#1368)
The five fully pass-through families (capture, system, admin, recording, observability) hand-wrote 'key: (options) => x.key(runtime, options)' per command plus a Bound* type mirroring RuntimeCommand -> BoundRuntimeCommand field-by-field. One mapped type + one generic binder now derives both. BoundOf keeps the optional-parameter ergonomics for commands whose options include undefined, so call sites like commands.back() are unchanged. bindAppCommands stays as a spread + manual 'list' override (filter normalization); selector/interaction families stay hand-written since most of their entries reshape signatures (prepended target/text positionals). |
||
|
Michał Pierzchała
|
0b0deb3b9e |
refactor(interaction): resolve the CapturedSnapshot name collision (#1371)
resolution.ts and selector-read-shared.ts (both in src/commands/interaction/runtime/) each exported an unrelated CapturedSnapshot type with a different shape. resolution.ts's export is never imported anywhere else, so rename it to InteractionSnapshot rather than force a merge with the richer, session-carrying shape that selector-read.ts/settle.ts/stable-capture.ts actually depend on. |
||
|
Michał Pierzchała
|
9a61ac71a1 |
refactor(cli): decompose runCli into explicit phase functions (#1373)
runCli inlined parse/help short-circuits, binding resolution, remote auth + materialization, four special-cased command kinds, dispatch, and a catch block coupled to ~10 closure-mutated let bindings across ~390 lines. Each phase is now a named function over an explicit CliRunContext: parseCliInputOrExit, resolveRunContextOrExit, runReactDevtoolsCli, resolveRemoteContext, buildClientConfig, maybeStartDaemonLogTail, createReplayReporterForTest, dispatchCliCommand, handleRunCliFailure. The context is mutated in place by resolveRemoteContext so the failure handler observes exactly the state the throwing phase saw — matching the previous closure semantics, including the close-with-no-daemon success path and daemon-log-tail-on-error. Behavior is unchanged; runCli itself is now ~75 lines of orchestration. |
||
|
Michał Pierzchała
|
19cea66c8b |
chore(deps): resolve dependabot alerts (#1372)
Pin transitive dependencies past their vulnerable ranges via pnpm overrides, scoped to the affected major so unrelated majors elsewhere in the tree stay untouched: - undici 7.24.7 -> 7.28.0 (root/website): @limrun/api pins undici to an exact version even in its latest release (0.44.0), so bumping the direct dependency can't fix this; override the transitive resolution instead. - shell-quote 1.8.4 -> 1.10.0 (examples/test-app) - js-yaml 4.1.1 -> 4.3.0 (examples/test-app) - brace-expansion 5.0.6 -> 5.0.7 (examples/test-app) - @babel/core 7.29.0 -> 7.29.7 (examples/test-app) - ws 7.5.10 -> 7.5.13 (examples/test-app) Resolves all 13 open Dependabot alerts (7 high, 3 moderate, 3 low). |
||
|
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
|
317469f35a |
chore: remove dead type re-exports, prune stale suppressions, gate unused-types (#1364)
* chore: remove dead type re-exports and gate unused-types Clears the 72 `unused-types` findings left by #1363 and flips `rules.unused-types` from `warn` to `error` so new ones cannot creep in. Every fallow detector is now at zero. 71 of the findings were re-export lines (`export type { X } from './y.ts'` with no importer of X through that path); one was a direct declaration (`HelpTopicName`, zero references anywhere). Removing them cascaded, which is the point: imports that existed only to feed a re-export became unused (8 of them, caught by `noUnusedLocals`), and clearing those exposed a further chain through `command-projection.ts` -> `batch/index.ts` -> `batch/projection.ts`. Also updated a comment in `batch-policy.ts` that described the `command-surface.ts` re-export path this removes, and dropped two `export type {}` husks left where every name in a block was dead. One finding was NOT what it looked like: fallow flagged `DebugSymbolsOptions` and `DebugSymbolsResult` in `apple/core/debug-symbols/types.ts`, but removing them broke `apple/core/debug-symbols.ts`, which re-exported them from there. Both ends were dead — every real consumer imports from `contracts/debug-symbols.ts` — so the fix was removing the intermediate re-export too, not restoring the leaf. Safety: the published type surface is unchanged. All 11 entry points in package.json#exports export exactly the same names before and after. Two chunk files differ only because `AndroidSnapshotBackendMetadata` relocated between internal code-splitting chunks, which is not a consumer-visible boundary. * chore: prune fallow suppressions that no longer suppress anything The whole point of this arc was that fallow looked clean mainly because of its own ignore list, so the list itself deserved an audit. Emptying `ignoreExports` and re-running shows most entries no longer match a real finding: 20 blocks collapse to 5, with no change in what either fallow invocation reports. Removed 8 fully-stale blocks. They went stale for three different reasons: - code moved (`assertSafeDerivedCleanup` is exported from runner-cache.ts, not the runner-xctestrun.ts the suppression named), - the exports became genuinely used (apps.ts, runner-contract.ts, runner-session.ts, cloud-webdriver.ts, the test-utils fixtures), - and `installAndroidInstallablePath` was suppressed in two places at once. `app-lifecycle.ts` needed a code fix rather than a suppression: it re-exported three `parseAndroid*` helpers from app-parsers.ts that nothing imports through it, so the re-export is dropped and the block goes away. The two type re-exports on that statement ARE consumed and stay. The seven `src/daemon/handlers/*` blocks are KEPT, collapsed into one documented glob. They are not stale: those handlers are reached only through the dynamic `import()` table in request-handler-chain.ts, which the --production analysis behind `check:production-exports` cannot follow. A first pass removed them because the staleness probe only exercised the default config; the repo runs fallow twice, and `check:production-exports` caught it. |
||
|
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
|
074ca140a2 |
test: serialize PATH-stub daemon/apple tests to end contention flakes (#1362)
`src/daemon/__tests__/runtime-hints.test.ts` and `src/platforms/apple/core/__tests__/index.test.ts` inject stub `adb`/`xcrun` binaries by mutating `process.env.PATH` and then spawn them, so each case waits real subprocess time. Run in the `unit-core` project at ~7x file parallelism they contend for CPU with every other stub-spawning file; a starved stub spawn pushes production down a generic failure path that returns a different error than the assertion expects. The failures present as assertion errors (not timeouts) and the failing subset shifts between runs — a pre-existing contention flake, reproducible on a clean tree. The repo already serializes the android scripted-adb tests into their own `fileParallelism: false, maxWorkers: 1` project for exactly this reason. This generalizes that project (renamed `android-adb` -> `subprocess-stub`) and adds the two files, so at most one real-stub-spawning file runs at a time across the whole suite. Assertions no longer let host wall-clock decide which error path runs. Option of injecting the spawn budget instead was not viable: runtime-hints passes no timeout to inject, and adding an exec-options seam purely for the test would be a test-only DI seam the CI gate forbids (AGENTS.md). Claude-Session: https://claude.ai/code/session_01NFepuH3Rh6ciRkvYJSCCtU Co-authored-by: Claude <noreply@anthropic.com> |