Skip to content

fix(ios): corroborate recorded tap outcomes - #1605

Merged
thymikee merged 7 commits into
mainfrom
agent/ios-tap-outcome-corroboration
Aug 5, 2026
Merged

fix(ios): corroborate recorded tap outcomes#1605
thymikee merged 7 commits into
mainfrom
agent/ios-tap-outcome-corroboration

Conversation

@thymikee

@thymikee thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

Partially addresses #1599 by corroborating an iOS XCTEST_RECORDED_FAILURE with one same-presentation post-action accessibility capture. A recent, comparable changed digest returns success with a warning; stale, keyless, unknown, unchanged, sparse, cross-backend, capture-failed, or presentation-mismatched evidence remains failure.

The comparison baseline is limited to five seconds and must declare the same presentation and snapshot backend, preventing unrelated screen drift or incompatible tree shapes from becoming a successful tap. Corroborated successes preserve target-v1 recording evidence and suppress the ordinary no-change retry so a later observation cannot dispatch the tap twice.

The change also resets process-bound snapshot penalty and private-AX state after external relaunch and makes the runner recovery hint honest. It removes the retry-spiral harm described in #1599, but does not claim to root-cause the underlying XCTest recorded-failure storm on the affected profile family.

Validation

  • Red-before proof: bypassing the freshness and key-presence guards made the stale-baseline test return success and made the keyless-baseline path perform a capture.
  • Focused corroboration suite: 11/11 tests passed, including stale/keyless/comparability fail-closed boundaries and the save-to-replay evidence chain.
  • pnpm check:affected --run: 126 files, 1,049 tests passed at 961c9f8fd.
  • iOS and macOS XCTest build-for-testing passed locally; exact-head native execution remains GitHub-authoritative.

Division of labor (#1599)

#1587 addresses the likely root cause: the tap false-failures were a downstream symptom of XCTest-channel saturation (grinding captures leaving abandoned work in testmanagerd), which the penalty/viewport/depth fixes removed — two live bsky-24 attempts and a 5-attempt relaunch/full-snapshot race on this head could not produce a single natural XCTEST_RECORDED_FAILURE. This PR is defense-in-depth for the residual: when the ambiguous failure does occur, one corroborating capture converts a landed tap to success-with-warning instead of a retry spiral. What remains open on #1599 narrows to confirming the saturation theory against the archived transcripts or a pre-#1587 build.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.97 MB 1.98 MB +5.7 kB
JS gzip 631.7 kB 633.0 kB +1.4 kB
npm tarball 761.6 kB 763.0 kB +1.4 kB
npm unpacked 2.67 MB 2.68 MB +6.1 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.6 ms 30.1 ms +0.5 ms
CLI --help 70.1 ms 67.3 ms -2.8 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/interaction.js +5.4 kB +1.3 kB
dist/src/runtime.js +217 B +78 B
dist/src/context.js 0 B -36 B
dist/src/registry.js -2 B +12 B
dist/src/session.js +12 B -4 B

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

P1: A corroborated iOS tap can return success while dropping the target identity required by recording/replay. buildRuntimeIosCorroboratedResponse rebuilds only ref/selector extras and calls finalizeTouchInteraction without recordedTarget; buildCorroboratedTapResponseData emits no selectorChain. A recorded click @ref therefore remains session-local without target-v1 evidence, so strict/repair publication refuses it or ordinary replay loses its pre-dispatch identity guard. Preserve the already-resolved runtime result/recording target across corroboration (or fail closed in recording/evidence paths), and add a regression that records, saves, and replays the corroborated tap—not merely an action-count assertion.

Exact-head iOS Smoke is also red waiting for Automation lab; classify/rerun it before readiness. The original hostile-screen false-negative path still lacks live forced-failure evidence.

@thymikee

thymikee commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

[P1] Avoid scheduling the no-change retry from the corroborated post-action snapshot.

The corroboration capture writes the changed after-state into session.snapshot, then this runtime success path calls finalizeTouchInteraction with the original coordinate pair and unchanged interactionOutcome.retryOnNoChange. markPendingInteractionOutcome therefore stores the after-state as its “pre” signature. The next observation can compare the screen with that same after-state, classify it as unchanged, and re-dispatch the tap that corroboration already proved landed—potentially twice.

Please suppress pending outcome retries for corroborated success and add a runtime coordinate regression with retryOnNoChange: true that performs a following snapshot and proves no second dispatch occurs. At the resulting exact head, also provide live iOS evidence for the changed path: one landed tap, success with the corroboration warning, a subsequent observation with no duplicate tap, fresh runner usability, and unchanged/sparse evidence remaining fail-closed.

@thymikee
thymikee force-pushed the agent/ios-tap-outcome-corroboration branch from 57b794f to 31fed27 Compare August 5, 2026 06:36
@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Not ready:

  1. The corroboration can fail open across snapshot backends. hasMatchingPresentation checks only presentationKey, which describes presentation flags but not SnapshotState.backend; compareCorroborationEvidence then treats any digest difference as success. If the baseline is xctest and the post-failure capture plan returns private-ax/another backend, backend-specific tree shape alone can convert an unchanged UI into a successful “tap landed” result. Require the same backend (or otherwise prove evidence comparability) and add a revert-sensitive cross-backend test that stays failed.

  2. There is no exact-head live evidence for iOS: XCTEST_RECORDED_FAILURE taps are false negatives on hostile screens — retry spirals burn the whole task budget #1599’s production path. The new tests mock dispatchCommand; the green iOS smoke lane never forces or observes XCTEST_RECORDED_FAILURE after a real tap. Before merge, reproduce or deliberately force the recorded-failure path on iOS and prove one landed tap returns the warning without redispatch, the runner remains usable, and unchanged/sparse evidence stays failed.

Also pin every documented fail-closed boundary—sparse post-capture, capture failure, presentation mismatch, and backend mismatch—and include red-before evidence in the PR validation.

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Reviewed against the #1599 field evidence (bsky-24: 3 cells, 2,600s burned on taps that landed but reported failure). The architecture is right — corroborate at the daemon boundary with exactly one capture, unknown stays failure — and the test suite is exactly the right paranoia, especially the retry-suppression case and the save→replay evidence-chain test. The relaunch state-clearing also closes a real gap #1587 left: the XCTest-channel penalty is bundle+TTL keyed (not PID-bound like the depth memory), so it genuinely leaked across external relaunches. Three findings:

P1 — claim strength vs evidence strength: any whole-tree digest delta against a possibly stale baseline converts failure into success. The baseline is session.snapshot with no freshness bound, and compareCorroborationEvidence treats any summarizeAxEvidence digest change as proof the tap landed. On the exact screens this targets (Bluesky feed/profile: autoplaying media, timestamps, async loads), an old baseline plus natural drift yields a false "tap landed" — and unlike a transient warning, a corroborated success is recorded into the session ledger and the saved .ad script with target-v1 evidence (your own replay test proves the persistence), so a false positive contaminates replay artifacts. Two tightenings, either sufficient: (a) bound baseline age (fresh-capture-or-skip; the agent loop's snapshot→tap pattern means a fresh baseline is the common case anyway), and/or (b) compare discriminating surface entries via the existing classifyBaselineSurfaceEvidence instead of the whole-tree digest — the real bsky-24 transitions (profile → image viewer) are wholesale surface swaps that discriminating comparison catches, while cosmetic drift does not. Same lesson as the #1601/#1606 review rounds: the gate for an agent-facing claim must be stronger than the gate for an internal heuristic.

P2 — keyless baselines default to "presentation matches" and then to a mismatched capture. hasMatchingPresentation returns true when baseline.presentationKey is absent, and captureCorroborationSnapshot then defaults to interactiveOnly: true — so a keyless full-tree baseline gets compared against an interactive-only capture, whose digest differs by construction → guaranteed false success on that path. Fail closed on a missing presentationKey instead of treating it as a wildcard.

P3 — "Closes #1599" slightly overclaims, per the same standard applied to #1603: the issue's second ask (why the restart path doesn't restore reliability — the tap-failure storm on the #1105 profile family) isn't root-caused here. The hint copy is now honest and corroboration removes the retry-spiral harm, which is the dominant cost — but the storm source remains. Suggest Part of + a narrowed follow-up for the underlying XCTest recorded-failure storm, or an explicit subsumption argument in the body.

Minor: corroboration adds one capture (~0.5s post-#1587) to every failed tap on hostile screens — worth a line in the ADR so nobody later "optimizes" it away without knowing the trade. Also note #1604 touches the same Lifecycle.swift/CommandExecution.swift regions — merge order will need a rebase.

🤖 Addressed by Claude Code

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Addressed the stale/keyless corroboration findings at exact head 961c9f8fd. Baselines now expire after 5 seconds, must carry a parseable presentation key, and must match both presentation and snapshot backend before any digest delta can corroborate success. Keyless evidence fails closed before capture.

Red-before, bypassing these guards made a stale baseline return success and made the keyless path perform a mismatched capture. Restored, the focused corroboration suite passes 11/11 and pnpm check:affected --run passes all 1,049 selected tests. The PR body now says “Partially addresses #1599” and explicitly leaves the underlying XCTest recorded-failure storm un-root-caused.

The remaining evidence limitation is also explicit: tests exercise the production daemon/provider route but mock the runner failure dispatch; exact-head live iOS CI does not deliberately force XCTEST_RECORDED_FAILURE. Exact-head CI is running.

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 961c9f8fd. Freshness, mandatory presentation-key, and backend comparability fixes are correct and revert-sensitive; no remaining code finding on those points. Still not merge-ready under the practical-device-evidence bar: tests mock runner dispatch and exact-head iOS smoke does not deliberately force/observe XCTEST_RECORDED_FAILURE. Before readiness, provide exact-head live evidence for landed tap → single corroborated warning success/no redispatch/runner usable, while unchanged and sparse evidence stay failed.

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Live-evidence attempt update: exact-head runner build succeeded and the dedicated ad-bsky-repro simulator contains Bluesky, but its XCTest runner is actively leased by another benchmark daemon (PID 8095, separate state dir). I did not terminate or steal that owner. Cloning is unavailable while the source simulator is booted; copying its app data into a spare simulator would transfer account/session material and was not authorized, so the temporary app install was removed and the spare simulator shut down.

All exact-head GitHub checks, including iOS Smoke, are green. The remaining blocker is specifically the independent review requirement for a live forced/reproduced production XCTEST_RECORDED_FAILURE path; it can be retried once ad-bsky-repro is released, without changing the code head.

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Exact-head live follow-up after releasing the stale lease:

  • Built and launched the 961c9f8fd5b371973f9a690c4d4a6e72002a68b6 iOS runner on simulator 1DBC007D-C481-40F0-A365-69241EB090B0.
  • Restored the Bluesky development build through a local Metro server and confirmed the production runner could snapshot the app.
  • The simulator is signed out. The archived bsky-24 issue evidence names only the benchmark transcript family; it does not include a restorable app-state fixture, account, profile URL, or setup command. Signed-out deep-link attempts remained on the landing screen, so the profile-wedge XCTEST_RECORDED_FAILURE route could not be recreated honestly.
  • All exact-head GitHub checks are green, including iOS Smoke. Local affected gates and the focused corroboration regressions are green with pre-fix red proof.
  • The temporary session, Metro server, daemon, and XCTest runner were cleaned up.

The implementation findings are resolved, but per docs/agents/pull-requests.md I am retaining one residual merge-readiness blocker: live evidence must observe the actual recorded-failure tap path. To close it, rerun the exact head on a signed-in Bluesky simulator restored to the bsky-24 profile screen, trigger a tap that returns XCTEST_RECORDED_FAILURE after landing, and capture the resulting success-with-warning plus the changed presentation evidence.

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 961c9f8fd after two bsky-24 simulator attempts. They provide useful hostile-surface evidence: semantic resolution failed closed on sparse/deferred AX, and a point press succeeded normally despite suppressed kAXErrorIllegalArgument snapshot issues. However, neither run emitted XCTEST_RECORDED_FAILURE or entered the corroboration branch. The changed recovery path therefore still lacks live evidence.

Not ready: deliberately inject or reproduce that runner failure on the exact head, then show one landed press returns the corroboration warning without redispatch, the runner remains usable, and unchanged/sparse captures remain failures.

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Reframing the missing reproduction (relayed from the benchmark-arc analysis; supersedes the previous blocker per review discussion):

Why two live bsky-24 attempts never emitted XCTEST_RECORDED_FAILURE: the field failure was a downstream symptom of XCTest-channel saturation, and #1587 removed the saturation. On 0.20.5 (the bench tool version), every capture ground the XCTest tree backend; slice-timeouts left abandoned grinds chewing testmanagerd, and taps dispatched into that wedged channel got failure-recorded while their coordinate activation still landed — the exact bsky-24 signature ("failed" tap, image viewer opens anyway). On this PR's head, the penalty routes hostile-screen captures to private-AX in ~0.35s, the channel sits idle, and taps simply succeed. The inability to reproduce is evidence for the causal story, not against the fix — but it does change what this PR is: defense-in-depth for a now-rare residual, not the cure for the storm.

Agreed merge gate:

  1. Compile-time, test-only failure-injection seam — a sibling of the existing systemModalProbeOverrideForTesting pattern (stored property under #if AGENT_DEVICE_RUNNER_UNIT_TESTS, never compiled into production; no environment flag). It records a production-shaped XCTest issue after performing the real tap, so the response path converts it exactly like the field failure.
  2. Deterministic live proof on the exact head, both directions: injected press on a real control → corroboration warning, screen demonstrably changed, no redispatch, runner serves the next command; injected press on dead coordinates → post-action capture unchanged → remains XCTEST_RECORDED_FAILURE.
  3. Field-shape supporting evidence: the relaunch/full-snapshot race — open --relaunch (which on this head clears the penalty and arms the warmup exemption, deliberately re-opening the grind window) → full non-interactive snapshot on the hostile feed (slice-abandoned ~8s) → immediate press on post media. Racy by nature; a handful of attempts reproducing one recorded-failure-that-landed is corroborating color, not the gate.
  4. Closes #1599Part of #1599, with the body stating the division of labor: perf(ios): halve hostile-screen capture cost under the XCTest-channel penalty #1587 addresses the likely saturation cause; fix(ios): corroborate recorded tap outcomes #1605 makes the residual ambiguous outcome honest instead of a retry spiral. The remaining open thread on iOS: XCTEST_RECORDED_FAILURE taps are false negatives on hostile screens — retry spirals burn the whole task budget #1599 narrows to confirming the saturation theory against the archived transcripts (or on a 0.17.6/0.20.5 build) rather than "root-cause unknown".

🤖 Addressed by Claude Code

thymikee added a commit that referenced this pull request Aug 5, 2026
…oboration (#1605 merge gate)

The field failure cannot be reproduced on this head: the tap false-failures
were a downstream symptom of XCTest-channel saturation, which the #1587
capture fixes removed. The seam records a real XCTIssue AFTER the real
gesture inside the per-command failure-count window, so
xctestRecordedFailureResponse and target invalidation fire byte-for-byte
like the field failure. Armed via a decrementing /tmp flag file (the daemon
regenerates tampered xctestrun templates, so env plumbing cannot reach a
daemon-spawned runner); compiled only under AGENT_DEVICE_RUNNER_UNIT_TESTS.

Live evidence on a daemon-spawned runner (Bluesky, ad-bsky-repro sim):
- landed case: injected failure on a real Search-tab tap -> success with
  the corroboration warning, screen verifiably on Search, no redispatch,
  runner serving next commands; flag consumed exactly once.
- unchanged case: injected failure on a dead-coordinate tap -> capture
  unchanged -> XCTEST_RECORDED_FAILURE preserved with the new honest hint;
  runner still usable.
- field-shape race (relaunch -> full snapshot -> immediate press, 5
  attempts): no natural recorded failure occurs on this head — the hostile
  tree needed for channel saturation is gone, corroborating the causal
  story.
@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Merge gate delivered in 5454517 — all four agreed points:

1. Compile-time injection seam (AGENT_DEVICE_RUNNER_UNIT_TESTS only; production compiles none of it). It records a real XCTIssue after the real gesture, inside the per-command failure-count window — so xctestRecordedFailureResponse and target invalidation fire byte-for-byte like the field failure. One implementation note: it's armed via a decrementing /tmp flag file rather than env — the daemon detects and regenerates tampered xctestrun templates (verified empirically: three armed templates were silently rebuilt), so env plumbing cannot reach a daemon-spawned runner without polluting production TS. The pure gate (tap-only, count-gated) is pinned by an in-bundle test; the consume path is proven live (flag observed 2→1→0).

2. Deterministic live proof, both directions, full stack (daemon-spawned runner, Bluesky on the repro sim, exact head):

  • Landed: injected failure on a real Search-tab tap → Tapped label="Search" (124, 817) with the corroboration warning, screen verifiably on Search afterwards, exactly one press dispatched, runner serving subsequent commands.
  • Unchanged: injected failure on a dead-coordinate tap → post-action capture unchanged → XCTEST_RECORDED_FAILURE preserved, with the PR's new honest hint ("was invalidated. Re-observe…"), runner still usable after.

3. Field-shape race (relaunch → full snapshot → immediate press, 5 attempts): zero natural recorded failures — expected and corroborating: with the dev-env backend down and #1587's fixes active there is no hostile tree to saturate the channel, which is precisely the causal story. The disease needs the grind; the grind is gone.

4. Body updated: Part of #1599 with the division-of-labor paragraph; what remains on the issue narrows to confirming the saturation theory against archived transcripts or a pre-#1587 build.

Gates: format/lint/typecheck clean, all 11 tap-outcome TS tests pass, runner test-build compiles, in-bundle gate test passed pre-rework on identical logic.

🤖 Addressed by Claude Code

@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed 5454517ca: the deterministic merge gate is sound. It is compiled only under the runner unit-test guard; the real tap executes before a real unsuppressed XCTIssue enters the existing failure-count conversion, so the daemon sees the production XCTEST_RECORDED_FAILURE path. Exact-head live landed/dead cases prove success-with-warning versus preserved failure, one dispatch, and next-command usability. The prior freshness/key/comparability and scope findings are resolved. Not merge-ready only because the branch is conflicting and the exact head consequently lacks full CI/native lanes: rebase while retaining both #1604’s text-input tests in CommandExecution and #1567’s recordedTargets semantics in interaction-common, then rerun full checks. The shared /tmp arming file is a minor non-production harness residual.

…oboration (#1605 merge gate)

The field failure cannot be reproduced on this head: the tap false-failures
were a downstream symptom of XCTest-channel saturation, which the #1587
capture fixes removed. The seam records a real XCTIssue AFTER the real
gesture inside the per-command failure-count window, so
xctestRecordedFailureResponse and target invalidation fire byte-for-byte
like the field failure. Armed via a decrementing /tmp flag file (the daemon
regenerates tampered xctestrun templates, so env plumbing cannot reach a
daemon-spawned runner); compiled only under AGENT_DEVICE_RUNNER_UNIT_TESTS.

Live evidence on a daemon-spawned runner (Bluesky, ad-bsky-repro sim):
- landed case: injected failure on a real Search-tab tap -> success with
  the corroboration warning, screen verifiably on Search, no redispatch,
  runner serving next commands; flag consumed exactly once.
- unchanged case: injected failure on a dead-coordinate tap -> capture
  unchanged -> XCTEST_RECORDED_FAILURE preserved with the new honest hint;
  runner still usable.
- field-shape race (relaunch -> full snapshot -> immediate press, 5
  attempts): no natural recorded failure occurs on this head — the hostile
  tree needed for channel saturation is gone, corroborating the causal
  story.
thymikee added a commit that referenced this pull request Aug 5, 2026
…oboration (#1605 merge gate)

The field failure cannot be reproduced on this head: the tap false-failures
were a downstream symptom of XCTest-channel saturation, which the #1587
capture fixes removed. The seam records a real XCTIssue AFTER the real
gesture inside the per-command failure-count window, so
xctestRecordedFailureResponse and target invalidation fire byte-for-byte
like the field failure. Armed via a decrementing /tmp flag file (the daemon
regenerates tampered xctestrun templates, so env plumbing cannot reach a
daemon-spawned runner); compiled only under AGENT_DEVICE_RUNNER_UNIT_TESTS.

Live evidence on a daemon-spawned runner (Bluesky, ad-bsky-repro sim):
- landed case: injected failure on a real Search-tab tap -> success with
  the corroboration warning, screen verifiably on Search, no redispatch,
  runner serving next commands; flag consumed exactly once.
- unchanged case: injected failure on a dead-coordinate tap -> capture
  unchanged -> XCTEST_RECORDED_FAILURE preserved with the new honest hint;
  runner still usable.
- field-shape race (relaunch -> full snapshot -> immediate press, 5
  attempts): no natural recorded failure occurs on this head — the hostile
  tree needed for channel saturation is gone, corroborating the causal
  story.
@thymikee
thymikee force-pushed the agent/ios-tap-outcome-corroboration branch from 5454517 to e4e69d1 Compare August 5, 2026 16:05
@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Rebased onto current main and force-with-lease updated exact head e4e69d1c5. Conflict resolution retains both #1604 text-entry regressions in RunnerTests+CommandExecution.swift and #1567 multi-target recordedTargets semantics, while keeping corroborated-tap retry suppression and the shared resolution-disclosure policy. Local validation is green: focused tap-outcome suite 11/11, pnpm check:affected --run 1,060/1,060 with bounded workers, and both iOS + macOS XCUITest runner builds succeeded. GitHub CI has been triggered; not waiting on it here.

@thymikee
thymikee force-pushed the agent/ios-tap-outcome-corroboration branch from e4e69d1 to 35a806b Compare August 5, 2026 16:07
@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Aug 5, 2026
@thymikee

thymikee commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

Re-reviewed exact head b25e3a0e0e. Its tree is identical to the previously reviewed ready head; the rewritten ancestry introduces no logic or test delta. The final reconciliation preserves resolution-disclosure behavior, the test-only landed/dead-coordinate injection proof, fail-closed corroboration boundaries, retry suppression, save→replay target-v1 evidence, and #1567 multi-target recording semantics. No remaining code finding; mergeable and conflict-free. All completed checks are green, with iOS Smoke still running.

@thymikee
thymikee merged commit d81ac0a into main Aug 5, 2026
31 checks passed
@thymikee
thymikee deleted the agent/ios-tap-outcome-corroboration branch August 5, 2026 16:38
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-05 16:42 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