fix(replay): bounded retry on pre-dispatch capture racing app launch (#1385)

[thymikee]

Summary

Fixes #1385: a step right after open --relaunch — e.g. a step-2 press — can have its pre-dispatch target-verification capture land while the app is still launching/mounting. That produces a transient capture-failed (Android's snapshot helper reports "insufficient foreground app content" mid-mount) or sparse-snapshot (iOS's private-AX fallback under load) verdict, which is not a real divergence, but the step still fails closed (identity-unverifiable) before ever dispatching.

• captureDivergenceObservation (session-replay-divergence.ts) now takes an opt-in retryLaunchRace flag. When set, an unavailable capture retries with a bounded backoff (fixed delay list, 12s wall-clock deadline) before giving up — mirroring wait's keep-polling landmark-identity semantics (Design read-only identity verification without breaking wait polling #1349) on this pre-dispatch path, still failing closed once the deadline passes.
• Only verifyReplayActionTarget's pre-dispatch verification capture (session-replay-target-verification.ts) opts in. The post-failure diagnostic capture (buildReplayFailureDivergence) and the post-resolution guard-mismatch capture stay single-shot, since those follow an already-real failure — and a number of existing unit tests deliberately stub a throwing snapshot dispatch there expecting an immediate result (repo convention: unit tests must not wait real time). Retrying unconditionally would have forced real wall-clock waits onto every one of those; I verified this by running the full suite before scoping the flag down to just the pre-dispatch call site.
• Amended ADR 0012 (decision 3) with the rationale, matching this repo's convention of documenting behavior-affecting replay fixes there.

Test plan

• Added two cases in session-replay-target-verification-runtime.test.ts: a transient capture failure that recovers within the retry window and still dispatches the action, and a persistent failure that still fails closed as identity-unverifiable once the bounded retry is exhausted.
• Full unit suite passes (4161 tests), no new slow-test-gate violations.
• tsc --noEmit clean.
• oxlint --deny-warnings clean.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.85 MB	1.85 MB	+596 B
JS gzip	592.7 kB	592.9 kB	+169 B
npm tarball	706.9 kB	707.1 kB	+163 B
npm unpacked	2.47 MB	2.47 MB	+596 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.9 ms	28.5 ms	+0.6 ms
CLI --help	58.0 ms	58.2 ms	+0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/session.js	+596 B	+169 B

[thymikee]

Reviewed (I filed #1385 and wrote the sibling wait-side fix in #1381, so this is a review from the adjacent trench). The shape is right: bounded retry at exactly the one call site that races a launch, opt-in so the post-failure and guard-mismatch diagnostic captures keep their immediate-failure semantics, ADR amendment matches the code (delay-list sum 11.8s coheres with the 12s deadline), and the two tests cover both the recover-and-dispatch and exhausted-fail-closed branches. Four findings, one substantive:

1. The retry classification is too broad — it retries permanent failures, not just launch races. captureDivergenceObservationAttempt's catch converts every thrown capture error into unavailable/capture-failed, and the loop retries on state alone. That includes conditions that cannot recover: Android snapshot helper is unavailable: the bundled helper artifact was not found (live-verified permanent during #1381's validation — it's what a worktree without built helper artifacts throws), device-offline, adb gone. Cost: ~12s of retries per annotated step before the identical divergence — a 10-step annotated script replayed in a helper-less environment goes from failing in seconds to +2 minutes of dead waiting. #1381 split this exact taxonomy for the wait loop: content verdicts (details.androidSnapshotHelperFailureReason — the "screen currently unreadable" class, retriable: true on the error itself) ride out; mechanism failures fail fast (isUnreadableCaptureContentError in snapshot-quality.ts). Suggest gating the thrown branch on that same predicate (duplicate it locally if you don't want to sequence behind #1381) so the two fixes keep one taxonomy: content verdict → retry, mechanism failure → immediate. The iOS sparse-verdict (non-throwing) branch retries rightly either way.

2. The sleep no-op mock is per-file, but the retry isn't. Any other test that drives an annotated step's pre-dispatch verification into a throwing or sparse capture now real-sleeps up to 11.8s and trips the slow-test wall-clock gate — several replay suites stub throwing snapshot dispatches (session-replay-target-guard, the repair suites). Worth a grep before merge; note that fixing (1) mostly dissolves this structurally, because generic throw new Error(...) stubs carry no content-verdict marker and would no longer retry at all.

3. Naming nit: retryLaunchRace names the motivating scenario, not the behavior — today it retries any unavailable capture. If (1) lands, the name becomes honest; otherwise retryUnreadableCapture says what it does.

4. Merge coordination with #1381: it restructures verifyReplayActionTarget around the same lines (post-resolution phase routing + an extracted recorded-unverifiable builder). Whichever lands second has a small mechanical conflict; on the merged result this flag's placement stays correct — the wait's post-resolution path never reaches this gate (it polls itself), and the path-1 recorded-unverifiable capture keeps no-retry, consistent with this PR's own opt-in rationale.

🤖 Addressed by Claude Code

[thymikee]

Addressed in 87c6fbf:

1. Retry classification too broad — fixed. The retry loop now only retries when the underlying failure is a content-quality verdict, not a mechanism failure: the non-throwing sparse-snapshot verdict still always retries, but a thrown capture-failed only retries when normalizeError(error).retriable === true — the same signal Android's helper capture path already emits for a content-poor/system-window-only rejection (rejectAndroidHelperContentUnavailable) and leaves unset for a permanent one (missing helper artifact, androidSnapshotHelperUnavailableError). Added a test proving a permanent failure fails on the first attempt.

2. Per-file sleep mock / other suites real-sleeping — confirmed dissolved. As you predicted, gating on (1) means a generic throw new Error(...) test stub (no retriable marker) no longer retries at all, so it's back to a single immediate attempt. Ran the full unit suite plus targeted runs of session-replay-target-guard, the repair suites, and the dispatch/runtime-failure suites — all green, no real-time waits introduced.

3. Naming nit — kept retryLaunchRace. Per your note, the classification fix makes the name honest again: it now only retries the class of failure a launch race actually produces.

4. Merge coordination with #1381 — noted, no code change here since #1381 hasn't landed; the placement (opt-in only at verifyReplayActionTarget's gate, not the post-resolution wait path or the recorded-unverifiable path-1 capture) should still be correct against the merged result per your read.

Also updated the ADR 0012 amendment to describe the retriable-vs-permanent gating.

🤖 Addressed by Claude Code

[thymikee]

Reviewed exact head 87c6fbf. Not ready. P1 — retriable is not a content-quality discriminator. The retry gate accepts any normalizeError(error).retriable === true. Android's helper wrapper deliberately re-lifts retriable from ADB mechanism failures (for example connection_dropped), so those failures consume the launch-race retry sequence despite the ADR saying mechanism failures fail fast. Add a narrow shared content-verdict taxonomy and a realistic retriable mechanism-error regression; do not reuse #1381's current broad androidSnapshotHelperFailureReason check. P2 — the documented 12s wall-clock cap is not strict. The deadline starts after the first capture, and after sleeping to the deadline the loop launches another unbounded capture. Bound capture by remaining budget or document/test a delay-only budget. Finally, exact heads #1386 and #1381 conflict in the ADR, target-verification code, and tests; reconcile their shared taxonomy/placement before merge. Completed checks are green; Linux smoke remains pending. No ready label.

[thymikee]

Addressed in 3b0e4db:

P1 — retriable is not a content-quality discriminator — fixed. You're right that androidSnapshotHelperCaptureError re-lifts retriable from the normalized underlying error, and adb's own transport-mechanism failures (connection_dropped, device_offline, server_version_mismatch in adb-executor.ts) are marked retriable: true for their own (adb-level, same-command) reasons — none of that is evidence of a content-quality capture verdict. Replaced the check with the narrow discriminator: the thrown error's androidSnapshotHelperFailureReason must be one of the exact three literals rejectAndroidHelperContentUnavailable attaches (empty-helper-output / system-window-only / content-poor-app-window, mirroring AndroidHelperContentRecoveryDecision['reason']) — never the free-form message androidSnapshotHelperCaptureError sets for a crashed/timed-out helper invocation, and absent entirely for a permanently-missing helper artifact. Added a regression test for exactly the adb-mechanism-failure-marked-retriable case you called out, proving it still fails on the first attempt.

P2 — the 12s cap wasn't strictly wall-clock, and the loop still launches an unbounded capture after the last sleep — addressed as a delay-only budget. Anchored the deadline before the first attempt (was computed after), so a slow first capture now eats into the same 12s rather than getting a free window on top. I did not add a per-capture timeout — that would mean threading a timeout/abort signal into captureSnapshot's whole call chain (Android helper capture, iOS runner calls, their own internal retries), which is a materially bigger change than this bug fix warrants, and each capture already carries its own platform-level timeout this loop isn't trying to re-implement or shorten. Instead I made the actual guarantee explicit and tested: the delay list bounds attempt count, the deadline bounds sleep time from entry, and neither claims to bound an individual capture's own duration. Added a fake-timer test (session-replay-divergence.test.ts) that simulates slow captures advancing the clock by 5s each and proves the 12s deadline — not the 7-entry delay array — is what stops retries (3 attempts, not 8).

#1381 merge coordination — unchanged, still just noted; no code change here since it hasn't landed.

Also rewrote the ADR 0012 amendment to match the corrected taxonomy and the delay-only-budget wording.

🤖 Addressed by Claude Code

[thymikee]

Re-reviewed exact head 3b0e4dbe. The prior retry-gate and deadline bugs are fixed with non-vacuous regressions, but the PR is not ready.

P2 — reconcile the runtime taxonomy with sibling #1381. session-replay-divergence.ts duplicates the three Android content-recovery literals in a local set. Typing the set against the union rejects invalid entries but does not enforce completeness when the union grows. #1381 already introduces the shared AndroidContentRecoveryReason enumeration and isUnreadableCaptureContentError; establish merge order/rebase and consume that shared predicate. The siblings overlap ADR 0012 and replay verification code, so ordering is material.

Readiness blocker — attach direct live validation. This changes device-facing replay behavior, but the PR reports only unit/static checks. Provide an exact annotated open --relaunch → immediate verified action run that demonstrates retry recovery and dispatch on a real backend, with cleanup. Android content-error and iOS sparse-verdict evidence are preferable because they enter through different classifiers.

All current checks are green.

[thymikee]

Merged main (#1381 landed) and resolved conflicts in commit c24611e:

• ADR 0012: kept both new Status bullets (Design read-only identity verification without breaking wait polling #1349's and Pre-dispatch identity verification races app launch: step-2 capture lands mid-transition and fails closed #1385's) side by side.
• session-replay-target-verification-runtime.test.ts: both sides had appended new tests at the same location — kept all of them (my 4 launch-race tests + feat(replay): recorded landmark identity for wait, is coverage — read-only step identity (#1349) #1381's 7 wait/is tests), no functional overlap.
• session-replay-divergence.ts: feat(replay): recorded landmark identity for wait, is coverage — read-only step identity (#1349) #1381 landed the exact taxonomy I'd duplicated locally (isUnreadableCaptureContentError in snapshot/snapshot-quality.ts) — replaced my local ANDROID_CAPTURE_CONTENT_QUALITY_REASONS/isCaptureContentQualityFailure with that canonical export, so there's now one shared content-quality-vs-mechanism-failure taxonomy instead of two parallel ones.
• session-replay-target-verification.ts auto-merged cleanly — confirmed the retryLaunchRace: true call site landed exactly where you predicted: after the wait's post-resolution branch (which returns before reaching it) and after the recorded-unverifiable path-1 capture (which stays no-retry via the extracted buildRecordedUnverifiableResponse).

Full unit suite green (108/108 on the directly affected replay/target suites, no new slow-test violations), tsc/oxlint/oxfmt clean.

🤖 Addressed by Claude Code

[thymikee]

Re-reviewed exact head c24611e. The shared #1381 taxonomy is now used, the wait/path-1 placement is correct, and the content-quality recovery, exhaustion, and retriable-ADB-mechanism fast-fail regressions are non-vacuous. All current checks are green. One readiness blocker remains: attach direct device evidence for an annotated open --relaunch → immediate verified action replay that actually takes the retry-and-dispatch path, with session cleanup. The issue’s pre-fix observations and unit tests do not prove the changed backend route.

[thymikee]

Attempted the requested direct live validation. Here's exactly what I did and what I found — I want to be upfront that it's a partial result, not a clean confirmation.

Setup: booted Pixel 9 Pro XL emulator + iOS iPhone 16 sim (avoided iPhone 17 Pro — its XCUITest runner was already owned by a concurrent #1349 validation session), installed/built examples/test-app (Expo dev-client), ran Metro locally, and drove everything through the repo's own CLI (node --experimental-strip-types src/bin.ts ...), never the Simulator MCP tool.

Recording a real annotated script: used open ... --relaunch --launch-url <dev-client deep link> --save-script + a real press on the Home tab bar to get a genuinely recorded target-v1 annotation (not hand-authored) — confirmed both platforms produce a real "step 1 open --relaunch, step 2 annotated press" script matching the issue's exact repro shape.

Replaying it (~25 attempts across both platforms, with --debug/--json): every attempt genuinely raced the launch — I got real selector-miss divergences (screen.state: "available", 0 matching refs) and once caught the dev-client's "Downloading 100%…" overlay mid-capture. This confirms the underlying phenomenon in #1385 is real and reproducible on live backends: the pre-dispatch capture does land mid-transition.

What I could not force: the specific capture-failed/sparse-snapshot classification this PR's retry gates on. In every live attempt, the capture mechanism itself stayed structurally healthy (available, just briefly content-less) rather than actually failing or coming back sparse — so retryLaunchRace never had anything unavailable to retry against; the divergence always resolved as a plain selector-miss (a different, pre-existing, correctly-unretried divergence class). I tried three ways to widen the window — natural timing, a React-level empty root render, a JS-thread-blocking synchronous delay at module load — plus deliberately spiking CPU contention further on an already heavily-loaded shared machine (load avg ~19 from concurrent sessions). None changed the outcome. My read: this test app's dev-client native chrome apparently always gives the accessibility backend enough to read validly, so the genuine mechanism failure (Android helper misclassification / iOS private-AX fallback) needs real backend-level instability, not just "JS hasn't rendered yet" — consistent with the issue's own framing of the iOS side as "flapping with machine load," i.e. non-deterministic even for the original report.

I reverted all the test-app instrumentation (nothing in this diff) and cleaned up: closed both sessions, killed the daemons, stopped Metro, force-stopped/terminated the app on both devices.

Given I couldn't force the exact condition live within reasonable effort, the strongest evidence I can offer that the retry gate is correct (not just that a race exists) is that the unit regressions use the real production error shapes verified against source — androidSnapshotHelperFailureReason: 'content-poor-app-window' from rejectAndroidHelperContentUnavailable (platforms/android/snapshot.ts), and the adb-mechanism-retriable case from the real classifyAdbFailure entries (adb-executor.ts) — not synthetic strings. I understand if that's not sufficient for your bar here; happy to keep trying a different forcing technique, or to hear if there's a known reliable repro (a specific device/app profile, cold-boot state, etc.) you used originally.

🤖 Addressed by Claude Code

[thymikee]

Merged main again and resolved conflicts in ea527a8. The only real conflict was in ADR 0012: main landed #1399's "rules-first" restructure (condensed the old per-PR bullet list under Status into a concise numbered "Rules at a glance" summary, full detail moved under Decision). Adapted #1385's entry to the new format — added it to the Status amendment list, extended decision 3's numbered rule with a one-clause summary, and kept the full > **Amendment (#1385)** blockquote (which auto-merged cleanly, unaffected by the restructure since it sits further down in the decision-3 body).

Everything else auto-merged cleanly (a large batch landed: #1401 cwd-scope fix, TV/Vega platform support, parameterized recorded inputs, doctor toolchain work, test-app additions). tsc/oxlint clean, all directly-affected replay/target suites green (97/97), full unit suite otherwise green modulo the same pre-existing contention flakes on this shared machine (runner-client/install-source/device-claims/request-router-open — none touch these files, confirmed unrelated).

🤖 Addressed by Claude Code

[thymikee]

Re-review at ea527a8: code review is clean, the ADR 0012 conflict resolution preserves the #1385 rule/amendment, the branch is mergeable, and current checks are green. The remaining blocker is validation of the changed route. The posted Android/iOS run reproduced launch timing and ordinary selector-miss, but explicitly never produced capture-failed or sparse-snapshot; retryLaunchRace therefore never executed on a live backend. Please attach a real-backend run that visibly retries one of those classifications and then dispatches the verified action, with session cleanup, or obtain explicit maintainer acceptance of that residual risk. Until then, do not apply ready-for-human.

[thymikee]

Ran a second, more targeted round of live attempts — still could not force capture-failed/sparse-snapshot on a real backend, and I want to lay out exactly what I tried so you can judge whether to accept the residual risk or point me at your original repro.

New technique 1 — WebView mid-mount race (iOS). Recorded a real 4-step script via --save-script: open --relaunch → press Settings tab → press "Open WebView accessibility lab" → annotated press on a link ("Jump to form") inside the WebView's injected content — reasoning that WebView accessibility-tree exposure is a documented source of genuine timing instability, unlike plain RN views. Replayed 5x: every run failed at step 2 (the Settings tab press, before ever reaching the WebView) with the same selector-miss / available / 0-refs pattern, the captured screen showing a live "Downloading 100%…" dev-client overlay. Tried a variant with --relaunch dropped from step 1 (in-app reconnect instead of cold app restart) — identical result.

New technique 2 — heavier in-app navigation race (Android). Recorded: open --relaunch → annotated press on the Catalog tab (a heavier list/search screen) → annotated press on a catalog item's "View details" button. Replayed 5x: every run again failed at step 2 with selector-miss/available/0 refs. A direct snapshot -i immediately after open --relaunch shows the identical underlying state each time: "Interactive snapshot is empty after filtering 40 raw Android nodes" — i.e. the Android helper's raw accessibility dump is never actually thin enough to trip content-poor-app-window (that requires the raw XML itself to be near-empty, not just filtered-empty), so this environment's dev-client scaffolding + emulator combination seems to always give the accessibility backend something structurally valid to report, however incomplete.

Across both rounds (~35 live replay attempts total, three different window-widening techniques, two navigation patterns, two platforms, deliberately spiked CPU contention) the outcome is 100% consistent: the launch/navigation race is real and reproducible, but it always resolves as a legitimate selector-miss on an available capture, never as the specific unhealthy-capture classification retryLaunchRace is gated on. I don't have a way to distinguish, from the outside, whether that's because this test app/environment happens to be more robust than whatever you hit originally, or because forcing it needs a real device rather than a simulator/emulator.

Given three rounds without success, could you either share the specific device/OS/app conditions from your original live reproduction (so I can try to match them), or make the call on whether the unit-level evidence (real production error shapes, verified against source) plus this now-extensive documented live-testing effort is sufficient to accept the residual risk? I don't want to keep re-spending review cycles on attempts that keep landing in the same place without new information.

Cleaned up again: closed all sessions, killed daemons, stopped Metro, force-stopped/terminated the app on both devices. No diff left in the repo (git status clean).

🤖 Addressed by Claude Code

[thymikee]

Rebased onto latest main (commit 4de8628, force-pushed) — no new maintainer feedback to address since the last round; still awaiting a response to the live-validation question above.

Note: I rebased rather than merged this time, since main had landed a large file-rename refactor (#1405, "put shared contracts below their consumers") since the last sync — a clean linear rebase avoids leaving that noise in this PR's diff. The three commits replayed with conflicts only in the same two files as before (ADR 0012's summary section, the shared test file where both PRs' new tests landed at the same location) — resolved each exactly as before, verified tsc/oxlint/oxfmt clean and all directly-affected suites green (97/97) after each step before continuing.

🤖 Addressed by Claude Code

[thymikee]

Direct answer: no, we don't have that repro. To be precise about the ~35 live attempts — the underlying launch-race timing is real and reproducible, but every single attempt resolved as a plain selector-miss (a structurally valid, if empty, capture). None of them ever produced the capture-failed or sparse-snapshot classification retryLaunchRace is gated on, so the retry-and-dispatch path never executed on a real device in any attempt. I reported that plainly rather than overstating it — the reviewer's read of the evidence is accurate.

The second finding — no targeted regression for the iOS sparse-snapshot branch — is fixed in fc60fd5. The existing tests only covered the Android thrown-capture-failed path (isUnreadableCaptureContentError); the non-throwing isSparseSnapshotQualityVerdict branch (always-retryable, no thrown error involved) had no test of its own. Added the mirror pair: a sparse verdict that recovers within the bounded retry and dispatches, and one that never recovers and fails closed as identity-unverifiable once exhausted. Both pass, tsc/oxlint/oxfmt clean.

The remaining gap is squarely the live-device evidence, which I was not able to produce despite the extensive attempts documented above. I'll leave the residual-risk acceptance decision to @thymikee.

🤖 Addressed by Claude Code

[thymikee]

Re-reviewed exact head fc60fd5b. The new iOS sparse-snapshot recovery/exhaustion tests are valid and non-vacuous, but the PR is not ready:

• P2: the rebase reintroduced a local ANDROID_CAPTURE_CONTENT_QUALITY_REASONS / isCaptureContentQualityFailure in session-replay-divergence.ts, importing an Android platform type instead of using the shared snapshot-quality.ts:isUnreadableCaptureContentError landed by feat(replay): recorded landmark identity for wait, is coverage — read-only step identity (#1349) #1381. ADR 0012 claims the same predicate is used; the duplicated typed set can silently drift. Replace the local set/predicate/import with the shared predicate.
• Readiness blocker: no real backend run has produced capture-failed or sparse-snapshot, recovered through retryLaunchRace, and then dispatched. The latest bounded Android run used the rebuilt helper and dispatched successfully, but captures stayed healthy, so the new route still did not execute live. Keep ready-for-human withheld unless a maintainer explicitly accepts that residual risk.

[thymikee]

P2 — fixed in 4a10b4e. You're right, and thanks for catching it — the branch's rebase onto main re-resolved the #1381 conflict without carrying forward my earlier merge fix, reintroducing the local ANDROID_CAPTURE_CONTENT_QUALITY_REASONS/isCaptureContentQualityFailure duplicate (with the Android-platform-type import) instead of the shared isUnreadableCaptureContentError from snapshot/snapshot-quality.ts. Removed the local duplicate entirely and consume the shared predicate directly, so there's one taxonomy again, matching what ADR 0012 already claims. All 65 tests across the directly-affected replay suites still pass, tsc/oxlint/oxfmt clean.

Readiness blocker (live capture-failed/sparse-snapshot recovery on a real backend) — unchanged, still open; see the earlier thread for the full account of what I attempted and couldn't force.

🤖 Addressed by Claude Code

[thymikee]

Re-reviewed exact head 4a10b4e. Code review is clean: the rebase regression is fixed by deleting the local Android taxonomy and consuming shared isUnreadableCaptureContentError; retry placement, Android/iOS recovery and exhaustion tests, mechanism fast-fail coverage, and ADR wording remain coherent. The branch is merge-clean and all checks pass.

Readiness blocker remains unchanged: no real backend run has produced capture-failed or sparse-snapshot, recovered through retryLaunchRace, and then dispatched the annotated action. The documented ~35 attempts only reached structurally valid selector-miss, so the changed route never executed live. Keep ready-for-human withheld unless that evidence is attached or a maintainer explicitly accepts the residual risk.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-27 11:34 UTC