mirror of
https://github.com/callstack/agent-device.git
synced 2026-09-14 20:06:34 +08:00
agent/fix-limrun-dependency-security
109 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]
|
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]
|
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
|
||
|
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
|
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
|
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
|
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
|
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
|
d45190613c | fix: report bundle sizes with two decimal MB precision (#1382) | ||
|
Michał Pierzchała
|
d237bc555d |
chore: remove verified dead code and migration scaffolding (~700 LOC) (#1367)
* chore: remove verified dead code and migration scaffolding Multi-agent audit of accumulated waste, every finding adversarially verified against call sites, git history, and the published surface before removal. Net -710 lines. - delete src/core/platform-descriptor/ (superseded ADR-0009 migration scaffold; parity tests now assert an inline table) - remove test-only seams: registry introspection exports, CommandFacet.extraDaemonWriters, MaestroEngineOptions.timing - remove dead flexibility: backend capability allow-list, screenshot-diff maxRegions, CloudWebDriverSupportLevel 'partial', clearFirst on the TS+Swift runner wire contract - remove dead deprecated surface: --session-locked / --session-lock-conflicts aliases (hard migration error now points at --session-lock), replay export --format single-value enum, unused Lease*Payload contract types, runtime-layer rotate duplicate - collapse pass-throughs/duplication: withRetry adapter, default-cloud-artifact-provider, connect-profile client-id hashing (3x sha256 impls -> one helper, byte-identical output), shared scripts walker, cloneValue -> structuredClone, fill-diagnostics moved into android/ BREAKING CHANGE: --session-locked and --session-lock-conflicts now fail with a migration error pointing at --session-lock; replay export --format is removed (Maestro was the only value); Lease*Payload types are dropped from the ./contracts subpath. * chore: satisfy fallow gates tightened by #1363/#1364 after rebase - drop the consumer-less AndroidFillVerificationNode re-export - reuse requireSnapshotSession in resolveSnapshotForRef instead of inlining the same authorized-frame resolution (fallow clone group); the helper's return type now guarantees the session it already throws for * chore: address review — keep cloud-webdriver partial capability metadata The partial/supported/unsupported levels and their notes are part of the lease-response capability contract for genuinely limited operations (Appium page-source snapshots, upload-then-install), not dead scaffolding. Restore them and the asserting tests unchanged from main. Also add the missing CHANGELOG entry for the Lease*Payload type removal from agent-device/contracts. |
||
|
Michał Pierzchała
|
c0fc822e80 |
chore: remove dead code and tune fallow's dead-code rules (#1363)
Evaluated knip (webpro-nl/knip) against the fallow setup already in the
repo, cleaned up everything it surfaced, then removed knip again: measured
head-to-head on the same tree, fallow is a strict superset once two
switches it already supports are flipped.
Dead code removed:
- `daemon/artifact-materialization.ts` (224 lines) had no production
caller, only its own test. Removing it exposed that
`downloadArtifactToTempDir` and the whole URL-fetch-with-redirects path
in `artifact-download.ts` were reachable only through it — the live
upload paths use the incoming-request helpers instead. That file goes
348 -> 123 lines. `readZipEntries` then fell out of `artifact-archive.ts`.
- Dead test-helper exports: 12 unused re-exports and 6 needlessly-exported
mocks in `session-test-harness.ts`, dead barrel entries in
`__tests__/test-utils/index.ts`, plus `withMockedXcrun`, `matchesSchema`,
`IOS_FRAME`, `IOS_TAB_FRAME`, `snapshotWithOffscreenContent`.
- `androidSnapshotHelperOutput` was duplicated byte-for-byte in
`provider-scenarios/android-world.ts`; it now imports the shared copy.
- 8 unreferenced type aliases, and 17 redundant type re-export lines in
`client/client-types.ts`. The published `.d.ts` is byte-identical before
and after all 19 files: those types already reach consumers through
`contracts/*` via `CommandResult<...>`, so this is not an API change.
Tooling:
- `.fallowrc.json` gains `includeEntryExports`,
`ignoreExportsUsedInFile: {type, interface}` and `unused-types: warn`.
That combination is what made the findings above visible; the previous
config was quiet mainly because of its own suppression list.
- Dropped 2 now-obsolete `ignoreExports` suppressions, added 3 documented
ones (published `sdk/*` surface, tool-config `default` exports, and the
`AssertTrue<...>` totality guards that exist only to satisfy
`noUnusedLocals`).
`unused-types` stays at `warn`: 72 pre-existing type re-export lines across
27 files remain, tracked separately. Every other fallow detector is at zero.
|
||
|
Michał Pierzchała
|
9efe445398 |
fix: target Maestro scrollUntilVisible container (#1338)
* fix: target Maestro scrollUntilVisible container * fix: align Maestro scroll viewport selection * fix: ignore hidden Maestro scroll containers * fix: derive Android Maestro scroll viewport |
||
|
Michał Pierzchała
|
d9f16de26f | docs: retire Maestro compatibility tracker (#1350) | ||
|
devin-ai-integration[bot]
|
5a33f8bfc7 |
feat(maestro): numeric option fields accept ${VAR} lookup interpolation (#1293) (#1342)
* feat(maestro): numeric option fields accept ${VAR} lookup interpolation (#1293)
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(maestro): validate resolved numeric strings before coercion
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(maestro): address review feedback on numeric resolution
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* fix(maestro-conformance): preserve unresolved ${VAR} numeric tokens in canonical model
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]
|
ab804340c8 |
feat(maestro): support optional on scrollUntilVisible and extendedWaitUntil (#1291) (#1339)
* feat(maestro): support optional on scrollUntilVisible and extendedWaitUntil Add optional support at command level and element level for scrollUntilVisible and extendedWaitUntil. The parser now accepts optional in both positions and propagates it to the command so the existing optional-command execution boundary downgrades a timed-out lookup to a warning and continues the flow. Update the upstream/076_optional_assertion divergence entry so only assertTrue remains unsupported, and keep the docs/support matrix in sync. Closes #1291 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(maestro): reject bare optional selectors, ORed visible/notVisible optionality, and add device differential scenario - parseMaestroSelectorMapEntries now rejects selectors that contain only optional and no real matching criteria, with rejection tests for scrollUntilVisible.element, extendedWaitUntil.visible, and .notVisible. - extendedWaitUntil now rejects simultaneous visible and notVisible conditions and derives optionality only from the single condition that will execute. - Add layer-3 differential flow/scenario optional-warned-scroll-and-wait that exercises both command-level and element-level optional on a missing target and verifies the flow continues to the final assertion. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(maestro): split parseExtendedWaitUntil to satisfy fallow complexity gate 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]
|
18b950407c |
fix(maestro): consume deferred stability before waitForAnimationToEnd (#1326) (#1337)
- Mark waitForAnimationToEnd as requiring a settled predecessor so it consumes the stability requirement parked by a preceding tap, swipe, or other mutation. Without this, the engine generation advances past the deferred requirement and the next tap's settlePending throws a generation mismatch. - Add a unit test that exercises tapOn -> waitForAnimationToEnd -> tapOn through the full daemon replay engine. - Add a layer-3 differential scenario for the same idiom. Closes #1326 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]
|
0f253f311c |
Maestro compat: support childOf on assertVisible/assertNotVisible (#1294) (#1334)
* Maestro compat: support childOf on assertVisible/assertNotVisible (#1294) - Accept childOf at command level in the Maestro IR and parser. - Thread childOf through the observation condition to the snapshot target resolver, reusing the existing ancestor-scoping path. - Project childOf into the conformance canonical selector so upstream/114_child_of_selector matches. - Remove the stale divergence declaration for 114 and update docs. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: regression-cover assertVisible/assertNotVisible childOf forwarding 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
|
d0227998d4 |
feat: add daemon stop lifecycle (#1323)
* feat: add daemon stop lifecycle * fix: harden daemon stop cleanup * fix: fail closed daemon stop cleanup * fix: bound daemon shutdown lease releases * fix: await active shutdown lease release * fix: release provider leases independently on shutdown |
||
|
Michał Pierzchała
|
a84caa8182 |
test: give the tap-retry differential a fixture control that forces the retry (#1327)
* test: give the tap-retry differential a fixture control that forces the retry tap-retry-if-no-change was parked in #1289 for being a coin flip: tapRetries measured 0 in run 29504440599 and 1 in 29510020718 with no change to the flow or commit. This re-adds it with a control that holds still, so the retry fires every run. The original diagnosis (a dynamic cart badge in the tapped title's subtree) had the right shape but the wrong scope. maestroSnapshotSignature hashes EVERY node on screen, not the tapped subtree, so no "static region" of the home screen could have worked. The actual coin flip is the gesture lab's remote image (reactnative.dev/img/logo-share.png): whether it lands before or after the tap decides whether the engine sees "changed" and skips the retry. So the fixture gets a dedicated inert surface — no state, effects, timers, images, or pressables — presented as a full-screen modal so iOS detaches the presenting screen and the tab bar's live badges leave the hierarchy too. On a real run it is 9 nodes against Settings' 55. Reached by a launcher on Settings via the Settings TAB, which is a deliberate choice twice over. A deep link would have kept the launcher out of every other screen's snapshot, but simctl openurl raises a SpringBoard "Open in app?" confirmation on iOS 26 even cold, and Maestro's openLink goes through the same path. And home's "Open settings" button sits below the fold, so reaching it needs scrollUntilVisible — the engine bug already waived under #1299. The flow carries no waitForAnimationToEnd: a navigating tap defers a stability requirement that the next tap settles before resolving its target, so the baseline signature is already captured on a settled screen. Adding one fails the flow outright, which is a real engine divergence filed as #1326. Verified on device (iPhone 17 Pro Max, iOS 26.2, Maestro 2.5.1): 10/10 consecutive differential runs ok, tapRetries [0,0,1] every run — the two navigating taps correctly do not retry, the inert tap retries exactly once. A single green run proves nothing here, which is the trap #1300 fell into. The parking guard in invariants.test.ts is replaced by three guards: the scenario stays active, carries its tapRetries invariant, and is never waived by a knownDivergence — a flaky scenario must be fixed, not declared. Fixes #1300 * chore: gitignore expo prebuild output in the test app Building the fixture app locally (what .github/actions/setup-fixture-app does in CI) runs expo prebuild and generates examples/test-app/ios/. It is generated and untracked but not ignored, so it shows up in git status and a `git commit -a` would sweep the whole native project in. Same for android/ when building there. Scoped to examples/test-app, so the repo-root android/ — which holds real tracked sources like android/ime-helper — is unaffected. Nothing is tracked under either path today, and the CI fixture-app cache key hashes src/**, app/**, modules/** and the config/lockfiles, so it does not reference these and is unaffected. |
||
|
devin-ai-integration[bot]
|
6d99914f49 |
feat!: remove deprecated gesture duration and rotate velocity inputs (#1218, #1216) (#1315)
* feat!: remove deprecated gesture duration and rotate velocity inputs (#1218, #1216) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: address CI failures - remove dead export, dedupe positional validation, migrate linux-desktop swipe test to pan Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fixup! preserve Maestro swipe endpoint-hold execution profile via internal seam Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs(adr): describe Maestro endpoint-hold internal seam in ADR 0013/0015 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * feat: surface Maestro swipe executionProfile in replay trace and assert endpoint-hold in differential 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
|
dd153a6233 |
fix(replay): default-exclude observation-only reads from repair heals, add --record opt-in (#1271 stage 2) (#1303)
* fix(replay): default-exclude observation-only reads from repair heals, add --record opt-in (#1271 stage 2) Amends ADR 0012 decision 6: snapshot/get/is/a read-only find are excluded from a repair-armed heal by default (session.saveScriptBoundary set), never from ordinary open --save-script authoring recording. wait keeps recording (flow timing, not observation). The corrective-read trap (wave-3 E3: the diverged step was itself a get) means blanket read-exclusion is unsafe, so a new --record flag forces one action through when the correction is itself a read. --record/--no-record are mutually exclusive (INVALID_ARGS if both are set) and are plumbed identically across CLI, the Node client, and MCP. The exclusion lives at the single daemon-side choke point (recordActionEntry/isExcludedRepairSegmentObservation), so an excluded read never grows session.actions.length -- the same counter the existing record-and-heal resume watermark (describeUnperformedRecordAndHeal) already checks, so the empty-segment fail-loud guard falls out for free (message updated to mention --record). Also fixes a latent bug found along the way: the get/is/find/snapshot CLI readers never forwarded --no-record/--record into the built request (only `open` did), so stage 1's "use --no-record" guidance was silently inert via the CLI. * test(integration): cover --record with a provider-backed repair-segment scenario (#1271 stage 2) The progress ratchet (test:integration:progress:check) flagged `record` as an unclassified public CLI flag. Classifying alone would only trade that failure for "missing Provider-backed integration workflow flag coverage" -- and the exclusions bucket is for config/output/transport flags, not behavior flags, so using it would dodge the ratchet rather than satisfy it. Adds a focused provider-backed scenario instead, next to the `--no-record` precedent in android-lifecycle.test.ts. It drives the real request router, session store, replay runtime, and script writer (only the ADB provider is faked), and proves the flag's actual purpose end-to-end: inside a repair-armed `replay --save-script` segment that diverged, the SAME `get text <selector>` runs twice differing only in `--record`; exactly one line lands in the committed healed .ad. Also asserts `--record` + `--no-record` is INVALID_ARGS. Verified the scenario reproduces the bug: with the exclusion neutered it fails on "a diagnostic read inside a repair segment must not be recorded". * fix(replay): key the repair-segment exclusion on provenance, scope --record (#1271 review) Addresses the maintainer review on #1303. P1 — the exclusion dropped PLANNED reads from the heal. It discriminated by command class, but the real discriminator is provenance. Replayed plan steps dispatch through the ordinary request path, so an authored get/is/find step hit the same recordIfSession -> exclusion path as an interactive read and never reached session.actions -- and the heal IS session.actions.slice(boundary). A repaired flow therefore replayed its authored `is visible` assertion and then silently dropped it from its own healed script: the heal quietly stops checking what it used to check, which for a 10x-QA-replay suite is the worst failure mode. Fix: an explicit provenance marker, not a heuristic. `internal.replayPlanStep` is stamped by invokeResolvedReplayAction -- the single point every plan step is dispatched, so it covers annotated and unannotated steps alike. `internal` is daemon-only (toDaemonRequest never copies it off the wire), so authored provenance cannot be spoofed; same channel as replayTargetGuard. The rule now lives once in isInteractiveObservation and both recording call sites consume it, so the mock fixture uses the production classifier instead of mirroring it. Planned observations survive automatically -- users never annotate their own .ad steps. --record is no longer a common flag: removed from COMMON_COMMAND_SUPPORTED_FLAG_KEYS, statically scoped via allowedFlags to snapshot/get/is, and validated dynamically for find (read-only allows; a mutating find click|fill|focus|type is INVALID_ARGS before any device work, sharing one isReadOnlyFindAction predicate with the read-only routing so the two cannot disagree). --no-record stays shared -- it applies to every recordable command. Removed from `open`, which is never observation-only. Rebased onto #1304 and dropped the four hand-rolled reader blocks. Split its helper rather than broadening it: noRecordInputFromFlags (all 13 readers) + observationRecordInputFromFlags (snapshot/get/is/find only). Two named helpers over one `allowRecord` policy arg -- the capability is then the helper's NAME, so a mutating reader physically cannot forward --record, whereas a policy arg would let a future mutating reader opt in by flipping a literal with no schema change. ADR-0012 decision 6 now states the provenance rule, not a command-class rule. The scenario gates the P1: its authored step is a distinguishable `is visible`, and it fails without the provenance check ("the authored 'is visible' step must survive the heal"). * test(daemon): pin that wire-supplied `internal` never reaches a daemon request #1271 stage 2 made `DaemonRequest.internal` semantics-affecting: `internal.replayPlanStep` decides whether an observation-only command is an authored plan step (kept in a repair heal) or an out-of-band diagnostic (excluded). That makes "internal means internally-stamped" worth pinning rather than leaving to convention. The invariant already holds, structurally and twice over: the boundary's `commandRpcParamsSchema` is an allowlist projection emitting only its eight named fields, and `toDaemonRequest` then builds the request field by field. Neither can carry `internal` off the wire. This posts a real JSON-RPC request carrying `internal: { replayPlanStep: true }` through a loopback server and asserts the dispatched request has no `internal`. Verified it fails ("a wire-supplied `internal` must never reach the daemon request") when both allowlists are regressed, so it guards the composite contract instead of restating one layer. |
||
|
Michał Pierzchała
|
856d5d4900 |
test: replace the hand-typed Maestro fixture with a generated conformance oracle (#1289)
* test: replace the hand-typed Maestro fixture with a generated conformance oracle Closes #1274. The old harness (scripts/maestro-conformance*) compared 5 hand-authored flows against a hand-typed transcription of Maestro 2.5.1's command model. It proved parser self-consistency, not conformance: all four bug classes that cost #1217 days of live debugging slipped past it by construction, and it verified no upstream SHAs despite parsing them. Every expected value here is generated from the pinned upstream artifacts. dev.mobile:maestro-orchestra:2.5.1 is published on Maven Central, so the harness runs the real parser and reads the real bytecode — no full Maestro source build. Layer 1 (parser): a Gradle/Kotlin harness drives the pinned YamlCommandReader over a corpus of 42 vendored maestro-test flows (sha256-recorded) plus authored bug-class, coverage, and invalid flows, capturing each parse. The verifier parses each flow with the live engine and classifies it identical / both-reject / we-reject / mismatch / we-are-lenient. Every non-identical outcome must be a declared divergence, so the 17 we-reject entries in expected-divergence.ts are the mechanical parity backlog (assertTrue, clipboard, travel, killApp, and option-level gaps) rather than silent drift. Layer 2 (semantics): ASM reads static-final constants straight from the pinned bytecode without initializing driver classes (MAX_RETRIES_ALLOWED=3, SCREENSHOT_DIFF_THRESHOLD=0.005, ANIMATION_TIMEOUT_MS=15000, erase cap, and the iOS pre-tap gate we intentionally omit), plus the parser-observed 400ms swipe default. Each is cross-checked against MAESTRO_COMPATIBILITY_PRESETS. Layer 3 (differential): scheduled device scenarios. Cross-engine comparison is outcome parity only and says so; finer behavior is asserted engine-side via invariants over replay-timing.ndjson. Bug class 4's detector — a tap must not consume the whole settle budget, since a full-budget tap means the stability loop never latched while the flow still passes — is pure and unit-tested against synthetic traces; only the device run is scheduled-only. regenerate.mjs verifies the pinned jar SHA-256s before trusting output and is byte-deterministic across runs. Layers 1-2 verify in normal CI via node --test with no Java (the job installs deps: unlike the layering guard it copies, the verifier parses with the live engine, which imports the `yaml` package). Acceptance: the four bug classes each have a fixture; every command in SUPPORTED_MAESTRO_COMMAND_NAMES (the parser's own dispatch table, now exported as the single source of truth) is corpus-covered or listed unverified; the five documented deviations are expected-divergence entries. * fix: address review findings on the conformance oracle P1 — layer-3 scenarios could never run. They pointed at layer-1 corpus flows, which exist only to be PARSED: they name a fictional com.example.app and elements that exist on no device. A device run would have failed before exercising any runtime behavior, making bug class 4's detector silently vacuous. Layer 3 now has its own flows under differential/flows/ driving the real fixture app (examples/test-app, com.callstack.agentdevicelab); the workflow builds and installs it and hard-fails if it is missing. A test enforces the separation so a scenario can never point back at the parse corpus. Nothing else in this repo builds or installs the Expo fixture app, so those steps are new and unproven. The workflow is therefore dispatch-only: the cron is removed until a supervised first run proves the path. A nightly job that fails at 05:00 every day teaches nothing. P2 — layer 3 installed whatever version the online installer served. It now pins MAESTRO_VERSION from pinned-upstream.json, so layer 3 cannot drift from the version layers 1-2 claim, and asserts `maestro --version` matches. P2 — fixture content was not bound to regeneration. CI compared only the embedded upstream metadata, so a hand edit to a captured command or constant passed: the transcription failure mode this oracle exists to remove. Two-layer fix, because per-PR CI must stay Java-free and cannot re-derive: - Each fixture now carries a contentHash seal that the verifier recomputes, so editing a capture breaks the build. Tamper-evident, and tested by actually tampering rather than assuming a hash comparison works. - New scheduled conformance-regenerate job re-runs the harness against the pinned jars and fails on any byte difference. Forgery cannot survive a real re-derivation. This is what makes "generated from upstream" enforced. P3 — boot-ios-test-simulator requires runtime-version; now passed alongside preferred-device-name, as the other iOS workflows do. * tmp: trigger layer-3 differential on this branch to prove the device path workflow_dispatch cannot run pre-merge (it registers from the default branch), so this temporary push trigger exists only to execute the never-run device path on the PR head and capture evidence. Removed before merge. * fix(ci): install the fixture app unfrozen for the layer-3 device run First live run of the device path failed at the very first step: ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. CI implies --frozen-lockfile and the fixture app's lockfile is out of sync with its package.json overrides. No CI job has ever built examples/test-app, so that drift was never surfaced. * fix: drop --ignore-workspace from test-app:install (defeats #649 security overrides) The first live run of the layer-3 device path failed at ERR_PNPM_LOCKFILE_CONFIG_MISMATCH, and the cause is a real latent bug rather than a stale lockfile. #649 moved the fixture app's `overrides` into examples/test-app/pnpm-workspace.yaml precisely because pnpm only honors overrides from a workspace root — they pin transitive deps (ws, brace-expansion, xmldom, postcss, uuid, shell-quote) to versions that clear Dependabot alerts. But `test-app:install` passes --ignore-workspace, which ignores that very file, so the overrides are dropped and no longer match the lockfile that has them baked in. It goes unnoticed locally because interactive installs are not frozen, and no CI job has ever installed this app. Dropping --ignore-workspace makes examples/test-app resolve as its own workspace root (it has its own pnpm-workspace.yaml and is not a member of the repo-root workspace), so the overrides apply and a frozen install succeeds. Verified both directions locally: with the flag + --frozen-lockfile reproduces the CI failure; without it, a frozen install completes and the lockfile's overrides stay intact. Note the workaround this replaces would have been actively harmful: installing with --no-frozen-lockfile resolves the mismatch by regenerating the lockfile WITHOUT the overrides, silently reverting the app to the vulnerable transitive versions #649 pinned away. * fix: make layer-3 scenarios prove what they claim, and parse the Maestro version Run 3 (29497919702) got the whole device path working: Expo build (30m), app installed, simctl check, pinned Maestro CLI install. Only the version ASSERTION failed — `maestro --version` prints an analytics banner before the version, and `tr -d '[:space:]'` mashed banner+version into one string. The CLI was correctly 2.5.1. Match the semver line instead, and set MAESTRO_CLI_NO_ANALYTICS (CI should not phone home). Verified the parse against the exact CI output: banner and clean forms both yield 2.5.1, wrong/empty still fail. tap-retry-if-no-change was vacuous: it tapped a navigating control, so the first tap always succeeded and retryIfNoChange never ran — it passed while proving nothing. It now taps the app's non-interactive title so the screen cannot change and the retry path is forced, and asserts tapRetries >= 1 from the trace (MaestroRuntimeMetrics already records it per step). A new metricAtLeast invariant kind carries the assertion; a test reproduces the old vacuity. percent-swipe no longer claims bug class 1. Truncation vs rounding is a <=1px delta that no app-observable device outcome can distinguish, so pass/pass could never back that claim up. The runtime half is instead pinned exactly by a pure unit test of resolveMaestroCoordinate (it short-circuits on a known viewport, so no device is needed) — verified to catch the regression by flipping trunc->round, which turns 3 of 6 tests red. Truncation had no test coverage at all before this. A test now forbids any device scenario from re-claiming bug class 1. * fix(ci): pass --maestro and match the fixture app's real UI in layer-3 flows Run 4 (29500262301) reached the differential itself — build, install, simctl check and the pinned Maestro 2.5.1 verification all passed — and surfaced two real bugs, both mine: 1. The runner invoked `agent-device test <flow>` without --maestro, so every scenario failed with "test does not support this file type". The repo's own scripts/run-test-app-maestro-suite.mjs passes it; the flag is what routes a .yaml through the Maestro compat engine. 2. settle-after-tap and percent-swipe assumed home-open-form is on screen at launch. It is not: real Maestro reported "Element not found: home-open-form", and the app's own helper flow scrolls it into view first. settle-after-tap now scrolls before tapping, mirroring that helper; percent-swipe no longer navigates at all and swipes the scrollable home screen, so it tests the conversion and nothing else. The remaining two flows already reported maestro=pass, so only the agent-device invocation was wrong for those. Note the settle invariant correctly reported "no-data: no completed tapOn steps" and FAILED rather than passing — a detector that cannot run is a failure, as intended. * feat: declare layer-3 divergences and schedule the differential Layer 3 ran both engines for the first time (29504440599) and immediately found a real engine bug. Blocking the measurement instrument on repairing what it just measured inverts the dependency, so layer 3 now gets the contract layer 1 already had: every divergence is a decision on the record. Adds `knownDivergence: { reason, tracking }` to the scenario type — the layer-3 twin of FLOW_DIVERGENCES. A declared divergence keeps the run green; only UNDECLARED ones fail. Two rules stop that from rotting, both enforced mechanically rather than by prose discipline: - `tracking` is required and must be a real issue URL (run.test.ts), because a declaration with nothing behind it is how "temporarily expected" becomes permanent without anyone deciding to. - a stale declaration FAILS: if a declared-divergent scenario starts passing, the run goes red until the declaration is removed. The fix PR must delete it, and the differential then enforces the gap stays closed — the oracle is the acceptance test for its own findings. Declared: - settle-after-tap -> #1299. Our scrollUntilVisible times out finding home-open-form where Maestro 2.5.1 scrolls to it and passes. Real engine correctness bug in an advertised command, found by this differential. Blocks bug class 4's device detector until fixed. - tap-retry-if-no-change -> #1300. The invariant caught the scenario being vacuous: both engines pass but tapRetries was 0, so retryIfNoChange never ran. Needs an inert fixture control; a scenario defect, not an engine one. Proven green on both engines and enforced now: percent-swipe, optional-warned-not-failed — the latter is real device-verified warned-vs-failed parity. With declarations in place the differential is green, so the schedule goes in (cron 05:00) per #1274. A green run still prints what it is not proving. * fix: park the flaky retry scenario instead of declaring it a divergence Run 29510020718 fired the stale-declaration guard on its first outing and caught my own mistake. tap-retry-if-no-change measured tapRetries=0 in run 29504440599 and tapRetries=1 in 29510020718 — same flow, same commit. So it is not vacuous as #1300 originally claimed: it is NON-DETERMINISTIC. The tap sometimes holds the hierarchy signature still and sometimes does not, because the fixture home screen carries live content. That exposes a real limit of the mechanism added in the previous commit: knownDivergence assumes the divergence REPRODUCES. A declared-but-flaky scenario flips between known-divergence (green) and stale-declaration (red) at random — a coin-flip scheduled job, which is worse than no scenario because it teaches people to ignore the differential. So the scenario is parked, not declared. The flow and the tapRetries invariant stay implemented and unit-tested, so the fix PR only re-adds the scenario once the fixture has an inert control. retryIfNoChange therefore has NO device coverage right now — tracked in #1300 and stated plainly rather than disguised by a green run. A test keeps it out of the active set until then. #1300 updated with the corrected diagnosis and both runs' evidence. Active differential: settle-after-tap (declared divergence, #1299), percent-swipe and optional-warned-not-failed (both enforced, pass/pass on real devices). * fix: make a knownDivergence waiver cover exactly one failure, not any failure P1 from re-review, and a real flaw: the code did not do what its own comment claimed. runScenario() collapsed every unexpected outcome and every invariant failure into `misbehaved`, then turned ANY of them green if the scenario carried a declaration. So while the #1299 scrollUntilVisible waiver is open, upstream Maestro could start failing too — or a different invariant could break — and the scheduled job would still report known-divergence and pass. A waiver for one bug was silently amnesty for the next. That is the exact failure this oracle exists to prevent, committed one commit after building the guard against it. knownDivergence now requires an `expected` signature: both engines' outcomes plus each declared invariant's status. The runner matches it exactly — - matches -> known-divergence (green, tracked) - misbehaves differently -> failed (red): not the failure the waiver covers - stops misbehaving -> stale-declaration (red): remove the declaration #1299's signature pins what runs 29504440599/29510020718 actually observed: maestro=pass, agent-device=fail, settle invariant no-data. Tests prove unrelated failures stay red under an open waiver: upstream also failing, our engine unexpectedly passing, a different invariant status, and a new invariant appearing are each NOT covered. A signature where both engines pass is rejected outright as describing no divergence. Also retains replay-timing.ndjson as a run artifact (review evidence note): the invariants are computed from that trace, so a report saying "tapRetries was 0" cannot be audited once the runner is gone without it. * perf(ci): cache the fixture app build for the layer-3 differential The differential job took ~30 minutes, of which 1331s (22 min, 79%) was building the Expo fixture app and only 347s was the differential itself — rebuilt from scratch on every run for an app that changes almost never. Cache the built .app, keyed on everything that can change the binary: the app's sources, native config, dependency graph, the build step itself, the iOS runtime, and the Xcode version. Mirrors the existing setup-apple-replay prebuilt-runner cache (same action pin, same Xcode-key + source-hash shape). On a hit the build is skipped entirely and the bundle is installed straight onto the booted simulator (~seconds), taking the job to roughly 8 minutes. On a miss it falls back to exactly the previous behaviour and repopulates, so the worst case is unchanged. The existing simctl verification still gates both paths, so a bad cache cannot produce a vacuous green: if the app is not installed, the job fails loudly rather than running scenarios against nothing. Note the first run after this lands is necessarily a miss. * refactor(ci): extract setup-fixture-app so any job can use the cached app The fixture-app build + cache was inline in the differential workflow, so nothing else could reach it. Extracted to a composite action mirroring setup-apple-replay, because the capability is what #320 has been missing: it wants replay coverage moved off Apple system apps onto a controlled fixture with stable ids, and that fixture (examples/test-app) already exists — CI just had no way to build and install it. The cache is genuinely shared. GitHub caches are per-repository and readable across workflows, and a run restores from its own branch or the default branch, so once a run on main populates it every workflow gets the hit and only the first one pays the ~22 minutes. The key is computed inside the action from a fixed input list and deliberately contains nothing caller-specific — folding a caller's workflow path into it would silently unshare the cache. Also removes a duplication risk: the action reads the bundle id from the built app's Info.plist rather than hardcoding it, so it cannot drift from what was actually built, and it fails loudly if the app is not installed. The conformance workflow keeps its own narrower assertion — that the installed id is the one its scenarios target — since that is its concern, not the action's. Usage: - uses: ./.github/actions/setup-fixture-app with: runtime-version: ${{ env.IOS_RUNTIME_VERSION }} # outputs: app-path, app-id, cache-hit * chore(ci): remove the temporary branch push trigger Run 29519848340 on this head executed both engines against the real fixture app and came back green, so the trigger that existed only to prove the never-run device path has done its job. Merged config is now cron (05:00) + workflow_dispatch, as required by #1274. known-divergence settle-after-tap maestro=pass agent-device=fail (#1299) ok percent-swipe maestro=pass agent-device=pass ok optional-warned-not-failed maestro=pass agent-device=pass This commit will not itself trigger a run: GitHub evaluates triggers at the pushed commit, and the push trigger is gone in it. |
||
|
Michał Pierzchała
|
117f78107e |
feat: add direct Limrun provider runtime (#1278)
* feat: add direct Limrun cloud runtime * refactor: reuse Android provider runtime for Limrun * refactor: pass runner context to provider runtimes * fix: remove Android gesture swipe fallback * fix: reconcile Limrun direct runtime with main * refactor: compose Android provider interactors in core * fix: satisfy packaged Limrun runtime checks * perf: load Limrun provider runtime on demand * docs: document Limrun device cloud flow * refactor: reuse Android reverse provider for Limrun * fix: isolate provider-owned iOS sessions * fix: preserve provider runtime boundaries * refactor: split close repair lifecycle * fix: reject unavailable provider leases * fix: reconcile provider runtime review feedback * test: stabilize alert deadline smoke assertion * fix: recover expired provider leases * fix: limit Limrun to remote simulators * fix: make Limrun provider cleanup durable * test: cover Limrun connect through CLI * fix: make provider expiry recovery durable * refactor: remove Limrun compatibility cleanup * fix: release live provider leases on expiry |
||
|
Michał Pierzchała
|
1a1ef7c419 |
feat(android): one persistent automation helper owning snapshot + viewport + canonical injection (#1281)
* feat(android): consolidate touch injection and gesture viewport into the persistent snapshot helper (#1275)
One Android automation helper now owns snapshot capture, gesture viewport
resolution, and canonical one-/two-pointer plan injection. A live persistent
helper session executes gesture/viewport commands over its socket protocol;
without a session the same APK runs one-shot via am instrument. The separate
one-shot multitouch helper APK is deleted (atomic replacement, no fallback).
Touch scheduling/injection is extracted into focused Java classes
(TouchPlan, TouchPlanInjector, PointerEventSchedule, GestureViewportReader)
instead of growing SnapshotInstrumentation. ADR 0013 amended.
* fix(android): stop a structurally-failed helper session before the one-shot viewport retry
A structured ok=false viewport response leaves the session process alive, and
Android permits only one instrumentation owner of UiAutomation - running the
one-shot fallback against a still-live helper contends with it and masks the
original structured failure. Stop the session first; regression pins that the
one-shot retry only executes once the session is gone.
* refactor(android): extract helper touch dispatch into focused classes; split session tests; document helper API v2 (PR #1281 review)
Addresses findings 2 and 3 from PR #1281 review (finding 1, viewport
session-stop ordering, was already fixed in
|
||
|
Michał Pierzchała
|
e58cbcdb5f |
refactor: colocate native platform sources under android/, apple/, linux/ (#1273)
Move the scattered root-level native projects into per-platform folders and drop
the now-redundant platform prefix:
- android-ime-helper/ -> android/ime-helper/
- android-multitouch-helper/ -> android/multitouch-helper/
- android-snapshot-helper/ -> android/snapshot-helper/
- apple-runner/ -> apple/runner/
- macos-helper/ -> apple/macos-helper/
- src/platforms/linux/atspi-dump.py -> linux/atspi-dump.py
Only repo source paths move. Identity surfaces stay frozen so no user's runner
cache is invalidated on upgrade: the derived-cache key hashes source paths
relative to AgentDeviceRunner and excludes packageVersion, and the
~/.agent-device/{apple-runner,macos-helper} namespaces, the
agent-device-android-*-helper artifact/manifest/protocol names, the
AgentDeviceRunner Xcode project, and the `prepare ios-runner` CLI command are
unchanged. Updates build/package scripts, CI, package.json files+scripts,
ignore/attr/fallow configs, runtime path resolvers, and test fixtures.
Also: re-base repo-root-relative refs inside the moved apple/runner for the
added nesting level (gated XCUITest fixture walk + two doc links), and clean the
legacy dist/apple-runner packaged output so the relocated runner can't
double-ship into the wholesale-included dist (with a regression test).
|
||
|
Michał Pierzchała
|
37895caf99 |
refactor: replace Maestro compat with typed direct engine (#1217)
* test: add pinned Maestro conformance harness * feat: add typed Maestro program IR parser * docs: define direct Maestro engine architecture * test: compare Maestro oracle with typed IR * feat: add direct Maestro program engine * refactor: narrow Maestro execution context * refactor: tighten Maestro program parsing * fix: verify iOS Maestro visibility waits * refactor: isolate retained Maestro runtimes * refactor: type Maestro target resolution * refactor: harden typed Maestro execution * refactor: share in-page swipe planning * feat: add typed Maestro runtime port * refactor: parse Maestro suite metadata from typed IR * refactor: centralize Maestro include loading * feat: execute Maestro files through typed engine * refactor: share replay built-in variables * fix: make Maestro target intent explicit * fix: refresh Maestro targets before input * refactor: format Maestro progress from typed IR * feat: compile typed Maestro replay plans * feat: bind typed Maestro runtime to public commands * feat: route Maestro YAML through typed runtime * refactor: remove legacy Maestro runtime * refactor: remove obsolete replay control model * refactor: split typed Maestro plan modules * fix: harden typed Maestro runtime semantics * docs: update direct Maestro architecture * fix: reconcile Maestro runtime with merged contracts * fix: harden typed Maestro execution boundaries * fix: harden typed Maestro runtime evidence * perf: avoid eager Maestro device resolution * refactor: finalize typed Maestro execution * fix: reject Android system-only helper snapshots * fix: preserve Android system dialog snapshots * fix: make helper-backed CI deterministic * refactor: invalidate Maestro observations before dispatch * fix: make Maestro selector policy explicit * refactor: remove Maestro ranking sentinels * refactor: make Maestro own observation stabilization * refactor: source Maestro compatibility presets * refactor: keep Maestro failure reports typed * refactor: simplify Maestro runtime policy * fix: isolate Maestro engine failures * refactor: consolidate Maestro swipe presets * fix: align Maestro selector and observation semantics * fix: preserve atomic iOS Maestro taps * fix: require semantic uniqueness for Maestro taps * fix: preserve Maestro parse provenance * docs: pin Maestro compatibility presets * docs: reconcile Maestro gesture viewport contract * perf: resolve Maestro gesture viewport directly * test: align Maestro replay regressions * fix: order Android gesture lift after endpoint * fix: settle Maestro gestures before continuation * fixup! fix: order Android gesture lift after endpoint * refactor: normalize Maestro swipes once * refactor: fail impossible Maestro observations * refactor: normalize Maestro defaults alias * test: reconcile Android provider scenarios * fix(android): synchronize single-pointer move events * test: align repair digest parsing * refactor: type Maestro runtime operations * refactor: keep Maestro controls compact * refactor: name Maestro diagnostic limit * fix: align Maestro parser and settling semantics * fix: complete Maestro compatibility semantics * docs: define Maestro compatibility boundaries * fix: refresh iOS runner target after relaunch * fix: reset prewarmed iOS runner after URL open * fix: preserve iOS Maestro target and swipe intent * fix: harden direct Maestro runtime semantics * fix: preserve ranked Maestro replay suggestions * fix: align maestro tap runtime semantics * fix: stabilize maestro ci contracts * fix: tighten maestro runtime architecture * fix: reconcile maestro replay with latest main * perf: tighten Maestro iOS stabilization * fix: preserve Maestro app lifecycle sessions * fix: restore Maestro CI coverage * fix: address Maestro engine review findings * refactor: consolidate Maestro compatibility internals * fix: scope Maestro target evidence to childOf |
||
|
Michał Pierzchała
|
f474f0784e |
feat: unify gesture planning and multi-touch execution (#1212)
* feat: unify gesture planning and multi-touch execution * fix: correct unified gesture helper behavior * refactor: tighten unified gesture architecture * fix: preserve gesture routing contracts * test: account for fresh gesture viewport * refactor: remove retired gesture series * fix: preserve example app navigation targets * test: reconcile unified gestures with helper ownership * docs: update Android helper gesture protocol * fix: refresh Maestro percentage swipe frames * refactor: remove stale Maestro frame cache * fix: harden unified gesture execution * fix: model gesture viewport in providers * refactor: remove legacy gesture paths * fix: remove unused swipe preset parser * refactor: tighten unified gesture boundaries * fix: close gesture review gaps * fix: preserve gesture compatibility contracts * fix: preserve multi-touch recording semantics * fix: refresh Apple runner state after app relaunch * test: lock Apple fling fallback route * fix: close Apple runner review gaps * refactor: tighten unified gesture seams * refactor: consolidate gesture planning policy * fix: preserve swipe response compatibility * fix: keep gesture lab aligned with replay coordinates |
||
|
devin-ai-integration[bot]
|
4e06304b12 |
fix: align layering claims, remove app-log ineffective dynamic import, refresh architecture records (#1222) (#1227)
* fix: align layering claims, remove app-log ineffective dynamic import, refresh architecture records (#1222) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: fix layering spine ordering (cli top), ADR 0009 deferred-scope wording, terse app-log-request-scope comment (#1222) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: show full rank groups in layering spine diagrams; clarify back-edge order vs literal imports (#1222) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: mark replay ADR context historical Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: refresh replay repair and app-log boundary status 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]
|
d585d74172 |
chore: close out architecture experiments (#1213)
* chore: close out architecture experiments Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: record unavailable live experiment Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * ci: make Android perf script atomic Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: explain atomic perf workflow Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: simplify back-edge diagnostics 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
|
e2bfed5f9f |
feat(replay): ADR 0012 migration steps 5+6 — resume + --update retirement (#1211)
* feat(replay): ADR 0012 migration steps 5+6 — resume + --update retirement
Step 5 (decision 4, resume): replay --from <n> --plan-digest <sha256>
resumes at a 1-based plan step, skipping 1..n-1 without executing them.
Every divergence report now carries a real resume object (allowed, from,
planDigest, reason?) computed by a preflight that rejects INVALID_ARGS
before any action when: the plan digest no longer matches the current
script (edits/includes/platform-conditioned expansion), --from is out of
range, a skipped step can produce outputEnv values, or the skipped range
or resume target is runtime control flow (retry/runFlow.when — these are
single plan entries, never individually addressable). `test` rejects
--from/--plan-digest both at the CLI-schema layer and at the daemon
dispatch layer (the original command name is only visible before test
rewrites its nested request to `command: 'replay'`).
New modules: src/replay/plan-digest.ts (canonical SHA-256 plan digest)
and src/daemon/handlers/session-replay-resume.ts (preflight + the
report's resume object), kept out of src/replay/ to avoid a
replay<->compat import cycle.
Step 6 (decision 1, retirement): --update/-u no longer rewrites .ad
files. The ADR mandates a no-op, not an error or flag removal: --update
now runs identically to a plain replay and returns the same bounded
suggestions every divergence already carries. Removed: healReplayAction's
retry-and-rewrite arm and its exclusive helpers (collectReplaySelectorCandidates
stays — decision 1's suggestions still use it), the write call from the
runtime loop, and the env/${VAR}-interpolation/compat-flow refusal guards
that existed only to protect that rewrite. writeReplayScript itself keeps
its own round-trip tests but is otherwise unused now; deleted after the
production-exports gate flagged it as dead.
Docs: cli-help.ts workflow topic + --update/--from flag help, AGENTS.md
selector pipeline note, maestro-compat-debt-map.md, website replay-e2e.md
and commands.md updated for the retired rewrite and the new resume loop.
* fix(ci): classify resume flags + provider-scenario resume coverage
The Integration Tests job's architecture-progress gate
(test:integration:progress:check) requires every public CLI flag to be
classified; --from/--plan-digest (replayFrom/replayPlanDigest) were
unclassified. Classify them as device-observable workflow flags and add
real provider-backed coverage to the Android lifecycle scenario: a full
replay diverges on a missing selector, the report's resume object is
asserted (allowed/from/planDigest), and resuming at the next index
replays only the tail. Also refresh the stale replayUpdate reason
("selector-healing replay update" -> the retired no-op).
* fix: bind replay resume digest to execution plan
* test: align replay runtime module topology
* fix: clear replay CI regressions
* docs: clarify replay repair and resume paths
* docs: clarify replay resume step semantics
* docs(replay): note that ${VAR} values stay out of the plan digest (ADR 0012 + workflow help)
Settled decision from the PR #1211 re-review (maintainer-approved): interpolated
${VAR}/--env/AD_VAR_* VALUES are deliberately NOT part of the resume plan digest.
Substitution happens after the digest is computed over the still-unsubstituted
${VAR} text, so re-running the same script with different variable values keeps
the same digest and stays resumable — supplying the right values on resume is the
caller's responsibility. The digest still binds the script/includes, the effective
--platform/--target, and per-action runtime hints + target-v1 identity. Documented
in ADR 0012 decision 4 and the `help workflow` resume topic.
* docs: clarify replay digest interpolation
|
||
|
devin-ai-integration[bot]
|
0a8ea3a57b |
refactor: consolidate architecture ownership and client results (#1210)
* refactor: consolidate architecture ownership and client results Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: keep selector parse chunk grouping current Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: update moved architecture breadcrumbs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: enforce moved selector architecture Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: keep selector guarantee ownership current Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: update selector ownership references Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
devin-ai-integration[bot]
|
952bc3704a |
refactor: keep command and daemon-route owner-file claims tooling-only (#1178) (#1192)
* refactor(command-descriptor): keep owner-file claims tooling-only Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(daemon): keep daemon-route owner-file claims tooling-only Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(daemon): guard against re-adding owner-file paths to the production route chain Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(command-descriptor): derive owner-file projection from colocated RAW_COMMAND_DESCRIPTORS - Keep ownerFiles on each RAW_COMMAND_DESCRIPTORS entry as the source of truth. - Add tooling-only __OWNER_FILES__ build flag so production bundles omit the ownerFiles properties entirely. - Derive COMMAND_OWNER_FILES from RAW_COMMAND_DESCRIPTORS instead of a hand-maintained parallel table. - Guard command-explain tests against leaking ownerFiles into production descriptor objects. - Enable treeshake.propertyReadSideEffects: false in tsdown to help drop the dead ownerFiles branch from production bundles. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: apply oxfmt formatting Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(build): guard tooling metadata exclusion Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(command-descriptor): drop global treeshake option and add bundle guard - Remove treeshake.propertyReadSideEffects from tsdown.config.ts; the __OWNER_FILES__ define + conditional spread already keeps owner files out of the bundle, so the global DCE lever is unnecessary and scope-creeping. - Add a comment on the __OWNER_FILES__ global declaration explaining the deliberate type-versus-runtime mismatch. - Add test/output-economy/owner-files-no-leak.test.ts to build dist and assert that no command or daemon-route owner-file path appears in the emitted JS. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(build): remove owner metadata property reads Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(command-descriptor): enforce owner claim totality 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
|
d4146c7f1b |
feat: add Android test IME helper for deterministic text entry (#1198) (#1201)
* feat: add Android test IME helper for deterministic text entry (#1198) Ships a headless InputMethodService (android-ime-helper) as a third Android helper APK, replacing the visible system keyboard during automated sessions. Renders zero accessibility nodes and accepts Unicode/CJK/emoji text over a base64-encoded broadcast channel, fixing both the settle-diff IME-chrome flood and the ASCII-only adb-shell text entry limit in one structural fix. - android-ime-helper/: InputMethodService + build/package scripts on the existing helper-APK toolchain (javac+d8+aapt2+zipalign+apksigner). - src/platforms/android/ime-helper.ts, ime-lifecycle.ts: install/version lifecycle (shared with the other two helpers via the new helper-package-install.ts), activation on session open, and on-device restore-hygiene (previous IME persisted to a device settings key so any daemon/state-dir can recover it; restored on close, daemon teardown, and daemon startup for orphans left by a crashed run). - input-actions.ts: fill/type route through the helper's broadcast channel when active, unicode-safe; unchanged ASCII-shell fallback otherwise. - doctor: new android-test-ime check flags a stuck helper IME with a copy-pasteable `adb shell ime set` remediation command. - Gating: default-on for emulators, opt-in via `open --test-ime` on real devices. - Dead-weight: rewrote the manual ADBKeyBoard workaround doc, dropped the now-provably-live skillgym non-ASCII eval case, updated the ASCII fallback's error message to point at the helper instead of dead-ending. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * fix(#1201 review): permission-gate the IME receiver, fix CI, add opt-out Addresses the independent review's blockers and should-fixes. SECURITY (blocker 1): the text-injection receiver was RECEIVER_EXPORTED with no gate — any co-installed app could inject text into the focused field while the test IME was active. Fixed by requiring the WRITE_SECURE_SETTINGS sender permission on the (in-process, dynamically-registered) receiver: adb shell holds it, third-party apps cannot. The reviewer's suggested exported=false + explicit-component approach was tried first but empirically breaks delivery on API 36 (adb shell cannot reach a non-exported receiver there) — documented in the helper README. Live-verified: a purpose-built rogue APK's broadcasts (implicit and package-scoped, no permission) are silently dropped, field unchanged; adb shell's bare broadcast still injects. Added ime-helper-security.test.ts asserting the permission gate and that no permissionless exported registration returns. CI (blocker 2): (a) added `testIme` to integration-progress-model flag buckets (Integration Tests was red on the unclassified flag). (b) mocked resolveAndroidImeHelperArtifact in session-doctor-android / ime-lifecycle / input-actions-test-ime tests so they no longer depend on android-ime-helper/dist existing on disk (Coverage was red on a fresh checkout); verified by running them with dist removed. Should-fixes: added `--no-test-ime` to opt out on emulators (tri-state gating, parser-tested); PR body's "byte-identical" claim corrected to size/CRC-match. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * docs(#1201): pin the API-36 exported-receiver constraint in a comment The RECEIVER_EXPORTED flag cannot express why it must stay exported. Add a one-line note so a future hardening pass doesn't switch to RECEIVER_NOT_EXPORTED and silently break the CLI (adb shell can't deliver explicit broadcasts to non-exported components on API 36+; WRITE_SECURE_SETTINGS is the actual gate). 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * fix(#1201 review): harden IME restore lifecycle (blockers 1 & 2) Blocker 1 — a failed restore no longer deletes the recovery value. restore now reads back default_input_method after `ime set` and only clears the persisted previous-IME record on a confirmed-successful restore; a failed set keeps the value so a later retry / startup recovery / doctor remediation can still un-strand the user off the helper IME. Blocker 2 — startup orphan-recovery no longer overwrites/races user state. It only restores when the device's CURRENT default IME is still our helper (so a user who legitimately switched away is left alone), and skips any device a live session in this process owns (the fire-and-forget startup vs. concurrent `open` race — activate now marks the device active BEFORE the `ime set`, so any recovery pass that could observe the helper active also observes the flag and skips). Never persists the helper itself as the previous IME. activate also verifies its own switch via read-back. Exported ANDROID_IME_HELPER_SERVICE_COMPONENT so restore compares the active IME without reading the packaged artifact from disk. Tests: failed-restore keeps the value (+ later recovery succeeds), startup no-op when current != helper, startup skips a live-owned device. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * chore(#1201): delete unused ACTION_ENTER path, baseline test-only export seams Rebased onto main (#1202 production-unused-exports gate). Two follow-ups: - Deleted the unused ACTION_ENTER broadcast end-to-end (TS sendAndroidImeHelperEnter + its test, Java handler, README): nothing routes through it — `keyboard enter` uses the keyevent ENTER path — so the new production-exports gate flagged it as dead production code. Removed rather than grandfathered. - Added the three legitimate test-only seams (resetAndroidImeHelperInstallCache, resetAndroidTestImeActivationCacheForTests, setAndroidTestImeActiveForTests) to fallow-baselines/production-unused-exports.json, matching how the sibling helper reset functions (resetAndroidMultiTouchHelperInstallCache, ...) are already grandfathered there. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * fix(#1201): stop daemon-startup adb spawn on non-Android hosts (macOS Smoke) Root cause of the red macOS Smoke shard (proven, not hand-waved): the fire-and-forget restoreOrphanedAndroidTestImeOnDaemonStartup ran `adb devices` at EVERY daemon startup, on every platform. GitHub macOS runners ship the Android SDK, so this cold-started the adb server mid-replay and destabilized the macOS System Settings replay timing — the failed job's cleanup shows "Terminate orphan process: pid (N) (adb)"; main's green runs spawn no adb. Fix: gate the startup orphan scan behind a host-side marker written in the daemon state dir when a session activates the test IME (mirrors the managed-web-browser orphan-cleanup `installed` gate). A host that never uses the Android test IME — the macOS CI runner included — never writes the marker and so never spawns adb at startup. The marker is cleared once nothing is left stuck. Adds SessionStore.resolveStateDir(); tests: startup recovery does not scan adb when no marker exists (+ marker cleared after a clean scan). 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * chore(#1201): suppress fallow class-member false-positive on state-dir accessor CI's Fallow audit flags SessionStore.resolveDaemonStateDir as an unused class member, but it is called via sessionStore.resolveDaemonStateDir() in session-open.ts — fallow's class-member tracer just doesn't resolve a method call sited inside a call argument. Renamed for clarity (avoids the collision with config.ts's free resolveStateDir) and added the localized fallow-ignore-next-line unused-class-member suppression. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> * fix(#1201 review): durable persist before switch + device-scoped recovery markers Addresses devin-ai-integration's two P1 restore-safety blockers on 19cbce79d. P1.1 — durably persist the restore target BEFORE the global IME switch. writePersistedPreviousIme now checks the `settings put` exit code AND reads the value back, returning a boolean. activate persists first and, if it cannot be persisted, fails open to the existing input path WITHOUT switching — a rejected `settings put` can no longer strand the user on the helper with no restore target. Regression test added. P1.2 — close the marker crash/offline blind spot. Recovery intent is now recorded per device, BEFORE the switch (ordering: durable record -> marker -> ime set), eliminating the post-switch/pre-marker crash window. Markers are device-scoped and each is retained until that device is actually observed clean: an offline/disconnected-but-stuck device keeps its marker and is recovered on reconnect instead of being cleared because the current `adb devices` scan saw no set-failed. Close-time restore clears only that device's marker (stateDir plumbed through teardown/close). Tests cover the persist-failure, post-switch/pre-marker crash, offline-then-reconnect, live-session-owned, and user-switched-away cases. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
devin-ai-integration[bot]
|
47134bf764 |
feat: add derived fail-open check:affected selector (#1195)
* feat: add derived fail-open check:affected selector Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor: simplify selector for complexity gate; add docs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: fail open on ambiguous non-source fixtures; guard catalog against real package.json/vitest.config Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor: use src/utils/exec.ts process helpers in check:affected runner Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(check:affected): SkillGym ownership, honest catalog, working-tree discovery - Add SkillGym ownership for skills/ and test/skillgym/; stop short-circuiting their Markdown as docs-only (findings 2 & 4). - Drop the fabricated GitHub 'SkillGym' job: it is a local-only gate, now localRunnable with no CI job, guarded by a workflow-existence self-test (3). - Fold working-tree (staged/unstaged/untracked) state into local discovery and disable rename detection so both rename paths classify (1). - Add run.test.ts entrypoint regressions (real diff/status/rename discovery, --run order/skip/stop-on-failure). Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(check:affected): union staged + unstaged diffs so they cannot cancel A single `git diff HEAD` nets index against working tree, so a staged add and an unstaged delete of the same file cancel and hide it. Collect `--cached` (staged) and unstaged diffs separately and union them; add a cancellation regression test. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(check:affected): cover required suite gates * refactor(check:affected): delegate tests to vitest --------- 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]
|
db492cbaed |
test(output-economy): add routine-workflow output-behavior oracle (#1190)
* test(output-economy): add routine-workflow output-behavior oracle Add a deterministic routine-workflow measurement (#1180) that pairs response bytes with follow-up behavior: fallback-observation count, retry count, and whether an actionable failure preserves the session. Refs chain across one recorded checkout session and counts derive from the real formatters, so dropping settled-diff refs, the unchanged- interactive tail, or a recovery handle fails the suite. Adds a matching non-gating help-conformance next-command case. Response defaults unchanged. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(output-economy): make routine-workflow ref-surfacing depend on rendered output Address review on #1190: - Drop the raw mutation-confirm/failure MCP data samples that leaked e4/e5 and mislabeled the surface; e4/e5 now surface only from the rendered CLI settled-diff, so dropping added refs genuinely raises the fallback count. - Track the failure once as its projection-invariant normalized payload (workflow.failure.shared.json) instead of duplicate cli/mcp raw copies. - Reuse the shared REF_TOKEN_PATTERN from economy-metrics.ts. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(output-economy): reuse shared fixtures in routine-workflow oracle Address review finding #2 on #1190: chain the workflow onto the shared per-surface fixtures instead of copy-pasting them. - orient/recheck reuse SNAPSHOT_RESULT + SNAPSHOT_DAEMON_RESULT; the first mutation reuses SETTLE_ADDED_REF_RESULT, so session identity and ref generations come from ./fixtures.ts and the two suites cannot drift. - Only the genuinely workflow-specific pieces remain local: the unchanged recheck, a tail retargeted onto a settled-diff ref (SETTLE_TAIL_RESULT taps an unsurfaced @e6 and can't chain), the in-session timeout failure, and its recovered retry. routine-workflow.ts drops ~110 LOC. - Rendered-output ref guard preserved: @e5 (settled diff) and @e7 (tail) surface only from formatter output; recovery semantics unchanged. 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]
|
ae74c51abd |
chore: add agent-efficiency regression guards (#1174)
* chore: ratchet architecture dependency graph Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: ratchet agent-facing output economy Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * feat: derive command navigation explanations Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: keep efficiency checks fallow-clean Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(layering): enforce back-edge ceiling monotonicity and cover root src files Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * ci(layering): pin back-edge-ceiling ratchet to PR merge-base Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(output-economy): baseline-independent actionability floors, policy-derived error, like-for-like screenshot surfaces Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(explain): resolve true CLI aliases, canonical usage, and derived owners Surface true CLI aliases from parser normalization (long-press, metrics, tap, launch, relaunch) distinct from catalog keys, preserving implied-flag semantics (relaunch => open --relaunch). Extract the canonical single-line usage builder to src/utils/cli-usage.ts so schemas without usageOverride include positionals and flags. Replace guessed handler paths with a completeness-checked daemon-route owner map keyed by the closed DaemonCommandRoute union, fixing silently-dropped non-kebab routes (reactNative, recordTrace) and generic dispatch. Add table-driven coverage for aliases, synthesized usage, split-family/route-variant/dispatch owners, structured output, and explain:command CLI exit/stdout/stderr. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: enforce exact ratchets and compact command explain Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: colocate command ownership metadata Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: bind daemon owners to production routes Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: preserve generic dispatch bundling Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: enforce monotonic output budgets 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
|
983625fc5d |
feat: fix codex runner, add --override-doc grading, port skillgym quiz cases to help-conformance bench (#1176)
* feat: fix codex runner, add override-doc grading, port skillgym quiz cases The 2026-07-09 evaluation of scripts/help-conformance-bench.mjs found it structurally right but broken for the codex runner (two bugs), thin on coverage (4 cases), sequential, and unable to grade a draft help rewrite without a rebuild. - Fix runCodex: (1) codex exec reads stdin until EOF when not attached to a TTY, and execFile never closes the child's stdin, so every codex call hung until RUN_TIMEOUT_MS with empty output — close stdin right after spawn. (2) `-o outFile` writes the same final JSON that codex also prints to stdout, so concatenating both produced two back-to-back JSON objects that broke every JSON.parse candidate and silently zeroed extractCommands() — prefer the clean -o payload, fall back to stdout only when it's empty. - Add `--override-doc <topicId>=<path>` (repeatable): loads a topic's text from a file instead of shelling out to `node bin/agent-device.mjs help <topic>`, so a draft help rewrite can be A/B graded with zero rebuild. - Port three cases from test/skillgym/suites/agent-device-smoke-suite.ts (settle-diff-is-observation, sample-output-settled-diff-next-target, sample-output-not-settled-needs-observe) as self-contained "next-command quiz" cases, generalizing the scorer to support regex matchers/forbidden patterns alongside the existing named expectations. Fixture output text matches the CURRENT settle rendering in src/commands/interaction/output.ts, including the "unchanged interactive (N):" tail added by #1167/#1172. - Parallelize the runner x case matrix with a concurrency cap (HELP_BENCH_CONCURRENCY, default 4); results still print in the original matrix order. - Extend test/skillgym/README.md's existing pointer to this bench with the new flags. Validated with real LLM calls (both runners, all 7 cases, 14 calls, ~$0.25 total): 13/14 pass; the one fail (claude-haiku-4-5 on dogfood-mode) is a genuine model miss (returned an empty command plan asking for the app name instead of committing to a generic plan), not a bench bug. `--override-doc` demonstrated live: stripping the dogfood doc's evidence-command examples regresses codex:gpt-5.4-mini from 3/3 to 2/3 on the same case, showing the flag both loads and changes grading. * fix: apply live-doc post-processing to --override-doc, fail fast on bad overrides Review findings on the initial version (all reproduced): - HIGH: an override for the --help:first30 doc id skipped the live path's firstLines(text, 30) cap, so a 49-line draft leaked lines 31-49 into the prompt — grading content a live run never shows, on the doc id every case uses. loadDoc now splits source (live shell-out vs override file) from post-processing, and the post-processing applies to both, so an override differs ONLY in where the text comes from. - MEDIUM: an --override-doc topic id no selected case uses was silently ignored (exit 0, real doc graded). Now fails fast listing the valid doc ids for the selection. - LOW: a missing override file threw a raw ENOENT stack trace; expected failures now print one clean Error line. Added --help usage text that documents last-wins semantics for repeated same-topic overrides and the post-processing parity. Guard tests (scripts/__tests__/help-conformance-bench.test.ts, wired into the unit-core vitest project by explicit path): a 49-line fixture whose prompt must keep line 30 and drop line 31, unknown-topic fail-fast with valid ids listed, clean no-stack error for a missing file, and last-wins for repeated overrides. All spawn the script in --dry-run with every required doc overridden, so they need no LLM calls and no built CLI. Live re-validation: a 33-line override of --help:first30 whose lines 31-33 instruct the model to emit a sentinel command; neither claude-haiku-4-5 nor codex:gpt-5.4-mini emitted it (both scored 4/4, matching the live-doc baseline), proving the cap applies end-to-end. |
||
|
Michał Pierzchała
|
888984169b |
fix: make record app-scoped by default (#1163)
* fix: reject recording for failed iOS simulator session * fix: make record app-scoped by default |
||
|
Michał Pierzchała
|
2717047b86 |
fix: normalize iOS simulator screenshot density (#1160)
* fix: normalize iOS simulator screenshot density * fix: avoid density metadata after screenshot downscale * fix: satisfy screenshot density CI gates * fix: harden screenshot metadata collection * refactor: centralize screenshot density policy * refactor: reuse screenshot density support check |
||
|
Michał Pierzchała
|
f18d0b2e92 | fix: improve settle observation guidance (#1154) | ||
|
Michał Pierzchała
|
7f61df30ae |
feat: add TV remote command (#1147)
* feat: add TV remote command * feat: improve TV remote ergonomics * test: cover tv-remote provider scenario * fix: preserve focused Android TV nodes * docs: tighten PR description guidance * fix: remove d-pad command alias * docs: clarify tv-remote hold syntax * feat: add tv-remote longpress CLI sugar |
||
|
Michał Pierzchała
|
ef4b66d4dc | test: remove slow-test ratchet pins (#1143) |