Skip to content

docs: ADR for interactive replay and resolution disclosure - #1177

Merged
thymikee merged 7 commits into
mainfrom
docs/adr-interactive-replay
Jul 10, 2026
Merged

docs: ADR for interactive replay and resolution disclosure#1177
thymikee merged 7 commits into
mainfrom
docs/adr-interactive-replay

Conversation

@thymikee

@thymikee thymikee commented Jul 10, 2026

Copy link
Copy Markdown
Member

Summary

Adds ADR 0012 documenting a decision set that emerged from several maintainer discussions, grounded in this week's benchmark and audit findings on .ad replay plus live hands-on replay sessions on the RN playground (iOS sim, both .ad and Maestro paths). Key quantitative evidence is inlined in the ADR (snapshots/interaction 3.67→1.00 with --settle, 14.3 vs 23.3/26.7 commands per task, 38/38 Maestro flows replayed in 539 s with zero model turns):

  • Retire --update healing as a silent actor. With an agent in the loop, a heal proposal costs one cheap turn to adjudicate — repurpose the existing candidate machinery (collectReplaySelectorCandidates, chain re-resolution) as ranked suggestions in the divergence report; --update never rewrites the script.
  • Disclose daemon-tree disambiguation and identify fast-path responses: an additive resolution field carries match count, winner, tiebreak reason, and at most 5 pre-action (non-actionable, non-pinned) alternatives on the runtime path; the direct-iOS fast path honestly reports not-observed; all six ADR 0011 dispatch paths get honest resolutionDisclosure cells.
  • Versioned .ad target-binding evidence (target-v1): identity is the recorded id (else normalized role+label) plus a leaf-anchored ancestry prefix (K=8, root-side truncation only); duplicates disambiguate by a genuine same-parent sibling child index, then viewportOrder scoped to the recorded scroll-region partition (one candidate domain at record and replay; unavailable regions never compared cross-region), with document order as the final deterministic tie-break — absolute geometry is demoted to never-compared diagnostics (no ±px tolerance). Replay classifies every annotated step through an exact six-path verification — matchCount (recorded-selector matches at replay, 0..N, always present) separates selector-miss (0) from identity-mismatch (≥1, no identity candidate); residual ties fail closed as identity-unverifiable with candidates listed, never a silent pick.
  • Structured divergence + digest-validated resume: a typed REPLAY_DIVERGENCE error preserved end-to-end (CLI/JSON/MCP, with error-path ref pinning), bounded and redacted per response level with overflow artifacts, step provenance (source file + line through Maestro runFlow flattening), and a replay-only resume requiring both --from <step> and --plan-digest <sha> from the report, with conservative variable/control-flow preflight rejection. Successful text replay prints a one-line summary.
  • Mandatory validation before benchmarks: matrix/provider contracts for all six disclosure cells, parser/writer round trips, all six verification paths (including signal-domain parity between record and replay), divergence bounds/redaction, resume digest rejection, MCP pinning, and --update no-write — the settle-benchmark replay arm (clean replay + one induced divergence) measures the economics only after those contracts pass.

Also adds the ADR to the docs/adr/README.md index.

This is a design-only ADR — no implementation changes are included. Every claim was verified against the current codebase (exact file/line citations throughout).

Test plan

  • git diff reviewed — docs-only change (new ADR file + README index row)
  • No markdown/doc lint gate exists in this repo (pnpm lint/check:* only cover src/test/config files), so nothing to run

thymikee added 2 commits July 10, 2026 09:02
… retiring --update healing

Records the decision to retire --update healing as a silent actor (repurposing
its candidate machinery as ranked suggestions), disclose selector
disambiguation in every interaction response, verify replay steps against
record-time identity evidence, and add an interactive replay --from loop with
a structured divergence report for all callers.
…ce for --from

Adds hands-on evidence from driving replay on the RN playground (silent
text-mode success, app-state divergence heal cannot fix, Maestro step-index
shift from runFlow flattening, per-format hint/code inconsistency, recordings
carrying zero observation steps), makes step provenance (source file + line,
including through Maestro runFlow inlining) a requirement of the divergence
report plus an optional replay --list-steps dry-run, and adds a one-line
text-mode success summary as decision 4d.
@thymikee

thymikee commented Jul 10, 2026

Copy link
Copy Markdown
Member Author

Architecture review of 67e1d04f. The direction is promising, but the ADR is not implementation-ready yet.

P1 - "every interaction response" conflicts with the accepted direct iOS path. Decisions 2-3 (ADR lines 111-131) assume daemon-side resolveSelectorChain yields a winning node and diagnostics. Simple iOS selector press/fill can succeed through the XCTest fast path without a daemon snapshot; ADR 0011 explicitly records disambiguation and response identity as waived gaps for direct-ios-selector (src/contracts/interaction-guarantees.ts:203-236). buildInteractionResponseData only composes fields supplied by that path; it cannot manufacture match count, alternatives, tiebreak reason, or id/role/label/rect. Specify one of: runner-side diagnostics plus parity contracts, disabling the fast path when disclosure/recording is required, or a deliberately narrower guarantee. Update the path x guarantee matrix and required contract tests accordingly.

P1 - the structured divergence report does not fit the current failure wire path. Lines 138-156 require actionable refs on a failed replay for all callers. Today replay enriches error.details; the client throws on ok:false (src/client/client.ts:89), MCP catches that and emits text only (src/mcp/router.ts:60), and ref pinning runs only on successful command results (src/mcp/command-tools.ts:75,168). The ADR must decide the exact daemon/client/CLI/MCP contract: whether divergence is typed success data with a nonzero CLI outcome or a structured error preserved end-to-end, how MCP exposes it, and how refs are pinned on that path. Define compact/default/full bounds and an explicit candidate cap, not only "capped."

P1 - recorded identity is not a specified backward-compatible contract. Lines 125-131 name id/role/label/rect but do not define grammar, escaping, optionality, normalization, or comparison semantics. Exact rect equality would create false divergences from layout/device/window drift, while restricting verification to ambiguous resolutions still permits a unique-but-wrong rebind. Also line 192 says this is not a format change while line 225 calls it one. Specify the plain-text representation and parser/writer round trip, old-reader/new-script and new-reader/old-script behavior, field stability/tolerance, and whether every resolved replay target is verified. This is target-binding verification, not general outcome verification.

P2 - replay --from needs execution-state semantics, not only app-state semantics. Skipping steps 1..N-1 also skips outputEnv values merged by earlier Maestro/runScript actions; later ${VAR} interpolation can fail even when the app is in the right state (session-replay-action-runtime.ts:128-130, src/replay/vars.ts:133-147). Replay/test share flag projection, and nested control-flow steps have fractional/repeated source-step identities. Define replay-only vs test exposure, 1-based executable-step vs source-line semantics, and either reject resume points with unsatisfied variable/control dependencies or provide a deterministic state reconstruction rule.

Before acceptance, validation should require focused unit/provider/MCP contracts for direct-iOS behavior, typed disambiguation payload and tie cases, identity round trips/backward compatibility, mismatch-before-action behavior, divergence error/ref propagation, response caps, --from variable preconditions, and --update no-write retirement. The proposed benchmark is useful economic evidence but cannot prove these contracts.

@thymikee

Copy link
Copy Markdown
Member Author

Re-review of fe8d7980: the first-round blockers are substantially addressed, but five implementation-contract gaps remain before ready-for-human.

  1. P1 - Decision 2's alternative refs are not actionable after a mutating interaction. The refs at ADR lines 160-164 come from the pre-action snapshot used by selector resolution, while the response arrives after press/fill may have invalidated that tree. Ordinary interaction responses neither issue a generation nor get MCP pinning; only snapshot/find and a post-action settle diff do. Treat winner/alternatives as explicitly pre-action diagnostic refs that must not be consumed, or capture and issue post-action refs with refsGeneration and MCP pinning. Require a mutation contract test. A target-binding divergence is different because the action was not sent, so its freshly issued report refs can remain actionable.

  2. P1 - the v1 identity tuple still cannot distinguish the motivating duplicate targets. Two siblings may share id/role/label/rect (the audit's duplicate-label, identical-rect case), so every recorded field can match even when replay chooses the other node. Require evidence to identify the winner uniquely at record/replay time, record stronger structural context, or classify an ambiguous set with multiple evidence matches as unverifiable divergence. Also define whether role means raw node.role or the normalized type used by selector construction.

  3. P1 - the proposed matrix guarantee is incomplete. ADR lines 175-179 classify only runtime-selector and direct-ios-selector, but ADR 0011's matrix has runtime-ref, native-ref, coordinate, and Maestro-fallback paths too. The completeness gate will reject a new resolutionDisclosure guarantee until every path has an honest cell. Specify all six classifications and require coverage for every enforced/delegated cell.

  4. P1 - --from N is unsafe if repair changes the expanded plan. N is an ordinal after includes/conditions/repeats expand, while the loop permits script edits before reusing it. An earlier edit, include change, or environment-dependent expansion can make the same N identify another action. Bind resume to a plan digest/token and validate it, or forbid plan-affecting changes. The divergence should expose resume: { allowed, from, reason, planDigest }, since output-producing/control-flow predecessors intentionally make some reports non-resumable.

  5. P2 - count caps do not bound or safely expose the whole divergence error. Action args, nested cause details, labels/selectors, and mismatch values remain unbounded; fill text may also be sensitive. Define per-field truncation/redaction and a serialized-byte ceiling per response level, with overflow written to an artifact path. Make screen evidence discriminated (available with refs/generation vs unavailable with reason/hint) so a failed or sparse snapshot cannot replace/mask the original replay failure.

Please extend mandatory validation for these cases. The revised direct-iOS provenance, additive .ad grammar, structured MCP error intent, replay-only flag, and conservative variable/control-flow rejection otherwise look coherent.

@thymikee

Copy link
Copy Markdown
Member Author

Final re-review of 8828470b: no actionable blockers remain.

The second-pass findings are resolved with enforceable contracts: resolution alternatives are bounded pre-action diagnostics rather than reusable refs; duplicate target evidence is structurally scoped and fails closed as unverifiable; resolutionDisclosure classifies all six ADR 0011 paths; resume requires a matching expanded-plan digest and reports whether it is allowed; and the structured divergence path now has field and byte ceilings, redaction, overflow artifacts, plus available/unavailable screen semantics. Mandatory validation names the corresponding matrix, mutation, parser/writer, replay, transport, MCP-pinning, and no-write cases.

All five docs-only checks pass. No device evidence is required for this proposed design-only ADR. Ready for maintainer judgment.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 10, 2026
@thymikee

Copy link
Copy Markdown
Member Author

Follow-up on the latest ADR: changing --update semantics under the same name is an accepted product choice, so I do not consider compatibility a blocker.

One substantive issue remains in the target-v1 contract. The fixed ±8 absolute-coordinate tolerance is likely to create false divergences from ordinary layout/window movement, while “same local identity” and ancestry truncation are not defined precisely. There is also a direct contradiction: matchCount must be positive, but it is required when zero nodes match. Please define the exact matching/truncation algorithm and either justify the geometry threshold with evidence or make geometry a normalized disambiguation signal rather than an unconditional identity field.

Non-blocking cleanup: make the benchmark/audit evidence durable and quantitative instead of pointing at ~/.agent-device-bench, and align the PR description (it mentions --list-steps and “one new flag,” while the ADR also requires --plan-digest).

Otherwise the ADR is strong: the six-path classification, direct-iOS honesty, actionable-ref separation, bounded/redacted divergence contract, and conservative resume preflight are well specified.

…unt, define matching algorithm

Reworks the target-v1 contract per review: identity is recorded id, else
role + normalized label, plus a leaf-anchored ancestry prefix (K=8, nearest
ancestors kept, root-side truncation only); absolute rects are demoted to
never-compared diagnostics with the ±8 tolerance removed rather than tuned;
duplicates disambiguate by recorded sibling order among the matching set,
then viewport-relative order within the recorded scroll region — never
absolute pixels; ties are identity-unverifiable divergences with candidates
listed. matchCount is redefined as the replay-time recorded-selector match
count (0..N, always present), with selector-miss (0) and identity-mismatch
(>=1, no identity candidate) as distinct classes in an explicit six-path
verification classification. Also inlines the quantitative benchmark numbers
(3.67->1.00 snapshots, 14.3 vs 23.3/26.7 commands, 38/38 in 539s) so the
evidence is durable without the external harness directory.
@thymikee

Copy link
Copy Markdown
Member Author

Addressed the 2026-07-10 08:30Z review in 1a31a646b, point by point:

1. target-v1 geometry demoted to a normalized disambiguation signal. The payload now has three explicit tiers: identity (recorded id when present, else normalized role + normalized label, plus the leaf-anchored ancestry prefix — compared exactly), disambiguation signals (sibling, then viewportOrder + scrollRegion — relative, never absolute pixels), and diagnostics (rect, never compared, carried only so reports can show where the recorded target was). The ±8 absolute tolerance is removed rather than tuned — the ADR states explicitly that no measured drift distribution exists to justify any constant, and that reintroducing an absolute tolerance requires measured evidence. The rationale you asked to encode is in the text verbatim: absolute rects are the least stable identity component (scroll offset, rotation, dynamic type, iPad/macOS window resize, RN layout shifts), and the audit's identical-rect "Prevent Remove" pair proves geometry cannot separate identical siblings even in the best-behaved case.

2. matchCount contradiction fixed. matchCount is now defined once: the count of nodes matching the recorded selector at replay time, range 0..N, always present in targetBinding. It no longer appears in the recorded annotation at all (the record-time payload carries only verification: "verified" | "unverifiable"), so the "positive but required at zero" contradiction is gone structurally. Identity verification applies only when matchCount >= 1; matchCount == 0 is the distinct selector-miss class (the repair is a selector repair), separate from identity-mismatch (>= 1 but no candidate passes identity). Both are spelled out in an explicit six-path classification (decision 3): recorded-unverifiable / selector-miss / empty identity set / verified / unique-but-wrong rebind / post-signal tie — with verified the only path that sends the action. Decision 4's kind enumerates the wire classes (action-failure, selector-miss, identity-mismatch, identity-unverifiable) and targetBinding.classification carries the finer path.

3. Exact matching/truncation algorithm. Now numbered and parameterized in decision 3: local identity = both carry id and NFC ids equal; else normalized roles equal and normalized labels equal (absent-on-both = equal, present-on-one = mismatch; a recorded id never matches a node without it). Normalization: NFC; labels additionally trim + collapse internal whitespace to one space; case-sensitive; empty-after-normalization = omitted by writer, absent to the comparator. Ancestry: nearest K = 8 ancestors as { role, label? } ordered leaf→root; truncation drops from the root side only (nearest ancestors always kept); comparison is a leaf-anchored prefix match (recorded chain must match observed position-by-position for its full length; shorter observed chain = mismatch; inserted/removed wrapper = identity change by design). Ties after all signals: out-of-range sibling ordinal falls through to scrollRegion-filtered viewportOrder; if neither isolates a member, the step is an identity-unverifiable divergence with up to 5 candidates listed — never a silent pick.

Non-blocking (a): the key benchmark numbers are inlined in the ADR's evidence section as a table (snapshots/interaction 3.67 → 1.00; commands per task 14.3 vs 23.3/26.7; 38/38 Maestro flows in 539 s at zero model turns), so the evidence no longer depends on the external harness directory. The identical-rect sibling repro was already inline in audit item (b).

Non-blocking (b): the PR description is aligned with what the ADR actually commits to: resume requires both --from <step> and --plan-digest <sha> (two flags), and --list-steps — which earlier drafts mentioned but the current ADR does not require — is dropped from the description rather than added to the ADR.

Validation section updated to match: the six verification paths (including out-of-range ordinals and scrollRegion filtering), leaf-anchored prefix matching with root-side truncation and inserted-wrapper mismatch, and rect-never-compared are now mandatory test cases.

…ord and replay

Fixes the P1 domain mismatch: sibling becomes a genuine same-parent child
index (parent already captured as ancestry[0], no new field; identical by
definition on both sides, non-isolating when the same index recurs under
different parents); viewportOrder gets one region-scoped domain — the
identity set partitioned by scroll region, ordinal within the recorded
partition on both sides, unavailable (never compared cross-region) when the
recorded region no longer exists; document order (pre-order index) is the
canonical total order making every ordering deterministic, including equal
rect centers. Residual ties stay identity-unverifiable with candidates
listed. Record-time write, replay verification, and mandatory validation
updated in lockstep; the six-path classification is unchanged.
@thymikee

Copy link
Copy Markdown
Member Author

Addressed the remaining P1 (positional signals used different candidate sets at record vs replay) in 6b5a1b2c7, point by point against the four enumerated consequences:

Different ordinal domains for viewportOrder → one region-scoped domain, identical on both sides. The identity set is now partitioned by scroll region (partition key = the local identity of a member's nearest scrollable ancestor, or none), and viewportOrder is defined as the winner's ordinal within its own partition at record time and looked up within the recorded partition at replay. The partition — never the whole identity set — is the ordinal's domain on both sides, so recorded and replayed ordinals always refer to the same candidate domain. When the recorded scroll region no longer exists at replay (empty partition), viewportOrder is unavailable and is never compared across regions; evaluation falls through.

scrollRegion ignored whenever sibling stays in range → sibling redefined so the concern dissolves. sibling is now a genuine same-parent structural ordinal: the node's zero-based index among its parent's children. The parent is already captured as ancestry[0] in the leaf-anchored chain, so no new recorded field is needed, and record and replay compute it identically by definition. It is region-independent on purpose — it constrains position within the parent, not the region — and it only denotes a candidate when exactly one member of the identity set carries the recorded child index; the same index recurring under different parents makes the signal non-isolating, falling through to the region-scoped viewportOrder. A same-identity node in another scroll region can no longer be accepted by positional coincidence: sibling requires structural uniqueness of the index within the identity set, and viewportOrder only ever compares within the recorded region's partition.

No deterministic final tie-break for equal rect centers → document order. A node's pre-order tree-traversal index is now the contract's canonical total order: equal rect centers, rect-less members, candidate enumeration, and candidate listing all resolve by document order, so every comparison in the chain identity → sibling → region-scoped viewportOrder → document order is total. A residual tie would require two nodes at identical positions in an identical tree — impossible under pre-order indexing — and the ADR states explicitly that even that residual case remains the identity-unverifiable divergence path with candidates listed, never a silent pick. Document order is an ordering tie-break, not a selection signal: exhausted/unavailable signals still end in identity-unverifiable, preserving the never-silent-pick invariant.

Lockstep updates. Record-time write (steps renumbered, partition definition added), replay path 6 (both signals restated over the shared domains), the three-tier signal description, and the mandatory validation bullet (same-index-different-parents, record/replay domain parity, region-gone unavailability, out-of-range ordinals, document-order determinism) all describe the same candidate sets. The six-path classification is unchanged — only the internals of path 6 tightened.

One consequence of the redefinition, now stated in the ADR: because both ordinals are computed from the winner over deterministic total orders, the record-time self-check succeeds by construction whenever the capture supplies the structural data, so verification: "unverifiable" at record time narrows to capture anomalies (e.g. missing parent linkage) and is kept as a fail-closed safety valve rather than an expected path.

One deliberate interpretation choice, flagged for visibility: the region-partitioned candidate base is the identity set (local identity + leaf-anchored ancestry prefix), not bare local identity — using bare local identity at replay would have introduced a fresh record/replay domain asymmetry of exactly the kind this fix removes.

PR description updated to match the tightened signal definitions.

… writer invariant, suggestion ranking contract
@thymikee

Copy link
Copy Markdown
Member Author

Addressed the four remaining items in 054682a (the previous worker session was interrupted mid-edit; this commit completes and extends its partial work):

P1 — matchCount on the recorded-unverifiable path: optional, key omitted (never null) exactly and only on path 1, which fires before any resolution; required 0..N on paths 2–6. No diagnostic-only resolution was added — a recorded-unverifiable annotation means there is no trustworthy recorded identity to resolve against, so a count there would invite misreading. The presence rule is stated once in decision 3 and mirrored verbatim in decision 4's wire contract.

P1 — migration dependency inversion: reordered into seven dependency-explicit slices. Divergence transport (report-only, kind: action-failure on the existing failure paths) now lands as step 2 with no dependencies — it's the immediately-useful provenance/evidence slice; annotations land inert as step 3; verification enforcement (step 4) consumes 2+3; resume (step 5) consumes 2 only and can ship in parallel with 3/4; retirement (step 6) depends on 2.

P2 — writer overflow: writer-parser invariant added — the recorder must never emit a payload its own parser rejects; deterministic root-side ancestry reduction until the 4 KiB ceiling fits; fail-closed downgrade to verification: "unverifiable" in the (arithmetically unreachable) terminal case; the record-time self-check runs against the reduced tuple so a verified claim is honest for exactly what was written.

P2 — ranking contract: decision 1 now defines a total order (identity-component strength > same scrollRegion > document order) with node-level dedup tagged by strongest match basis; digest level omits suggestion entries but carries suggestionCount; --update --level digest is resolved explicitly (levels affect report content only, never file behavior — legacy rewrite until step 6, no-write + suggestions after). Validation section references both.

One additional fix from a holistic pass: the interim state of the P1 edit had introduced a decision-3/decision-4 contradiction — the wire contract still said targetBinding.matchCount is "always present" for all three binding classes, while identity-unverifiable can arise both from path 1 (no resolution, no count) and path-6 fall-through (count present). Decision 4 now states the conditional rule and also pins targetBinding.classification to always equal the top-level kind.

@thymikee
thymikee merged commit 66fe801 into main Jul 10, 2026
5 checks passed
@thymikee
thymikee deleted the docs/adr-interactive-replay branch July 10, 2026 09:39
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-10 09:39 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant