fix(android): back off a timed-out snapshot helper session and bound content re-captures

[okwasniewski]

Summary

From e2e feedback: Android snapshots on a hung app could run for about 91s and time out, and a helper failure was sometimes reported as "output could not be parsed".

• Any failed persistent helper session capture (timeout, dead socket, protocol or helper error) tears the session down, and that helper build stays one-shot for the fallback budget plus start backoff (at least 40s). It is no longer respawned to fail again on every attempt and command.
• Content re-captures do not start after a 10s window, checked after the re-capture delay too. Worst case once the helper is installed is about 70s. A first-run helper install (30s budget, inside the same request, cached per device after) can take that one attempt to about 87s, near the 90s request envelope; it already closes the window, so no re-capture follows.
• When am instrument exits 0 but the helper reports a failure, that failure and its typed reason are kept instead of the parse error.

8 files touched, all in platform-android.

Validation

Tested commit: 12757c4 (on upstream/main 9a245d0).

• Targeted: snapshot*.test.ts in platform-android: 155 passed. New window test fails without the delay recheck.
• pnpm check:affected --run --base upstream/main: all runnable checks passed (385 files / 2641 tests).
• Live, Pixel_9_API_37 emulator (pre-rebase head) with Settings SIGSTOPped: after the failed session capture, the next command skipped the session instead of respawning it. After SIGCONT, snapshots took 0.2-0.9s.

Risks: the original 91s timeout and the exit-0 helper failure were not reproduced live; those paths rest on unit tests. First-run install worst case sits near the envelope.

[Copilot]

Copilot review overview

🟡 Changes recommended

The retry delay can cross the deadline and still start another full capture.

Review effort: Balanced
Findings: 1

Open (1)

• Recheck deadline after delay before starting helper capture · New

What changed in this PR

Bounds Android snapshot retries and preserves meaningful helper failures.

Changes:

• Adds failed-session teardown and retry backoff.
• Limits content recaptures to a 10-second window.
• Preserves structured helper errors and adds regression tests.

File	Description
snapshot.ts	Adds the recapture deadline.
snapshot-helper-session.ts	Retires failed persistent sessions.
snapshot-helper-session-lifecycle.ts	Tracks failed-session backoff.
snapshot-helper-capture.ts	Preserves structured helper failures.
snapshot-helper-session.test.ts	Updates session fallback expectations.
snapshot-helper-session-lifecycle.test.ts	Tests timeout backoff behavior.
snapshot-helper-capture.test.ts	Tests zero-exit helper failures.
snapshot-content-recapture.test.ts	Tests bounded content recaptures.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[cubic-dev-ai]

All reported issues were addressed across 8 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[thymikee]

Code review of b0a0820 found no blocking problems in the backoff and bounded re-capture change. Smoke Tests was still pending when I checked. That job likely runs the Android snapshot route this diff changes, so it must finish green before merge. I found no conflicts.

Not blocking, take or leave: the backoff in snapshot-helper-session-lifecycle.ts applies to any session capture failure, with a minimum of 40s from the fixed fallback budget, so should it key on the measured fallback time or only on timeout reasons (or should the PR text state the any-failure scope)? The test at snapshot-helper-capture.test.ts could also assert the typed capture failure reason detail, not just the message text. And snapshot-helper-session.ts could pass session.identity to the retire call instead of re-reading the session map.

The deadline thread on snapshot.ts:310 still applies, and it is a small fix to make or dismiss: the deadline is checked before the 250ms delay, so a re-capture can start about 250ms past the window. The matching cubic P2 thread says the same, so one fix covers both. Two cubic P2 threads do not apply, so please resolve them. One assumes concurrent captures on one device, but the daemon router serializes same-device commands. The other assumes an overlapping acquire during teardown, and the same router rule prevents it.

The live Pixel_9_API_37 run appears only in the PR body, on the pre-rebase head, with no logs. The 91s hung-app timeout and the exit-0 helper failure were not reproduced live, so those routes rest on unit tests. The worst-case total also leaves out the 30s helper install budget, so is install inside the same request?

[Copilot]

Copilot review overview

🔵 Needs a closer look

A teardown timeout can bypass recording the failed-session backoff, allowing immediate respawns.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Recheck deadline after delay before starting helper capture

Previously missed (1)

In code that hasn't changed since last review

Record failed session backoff when cleanup times out

packages/​platform-android/​src/​snapshot-helper-session-lifecycle.ts:256

If teardown times out while removing the forward, stopAndroidSnapshotHelperSession rejects (its cleanup uses Promise.all), so execution never reaches failedSessions.set. The failed helper can then be spawned again on the next command, defeating the new backoff specifically when cleanup is unhealthy. Record the backoff in a finally block while still excluding caller cancellation.

[okwasniewski]

[claude-opus-5-5] responding on behalf of Oskar

Pushed 12757c4.

Changes:

• 60446b3: re-capture deadline is checked again after the 250ms delay, and the request is rejected with the previous attempt's decision. New fake-clock test (9.9s capture) fails without it. 12757c4 moves the loop into its own function so it stays under the fallow complexity limit.
• da59b7a: retire takes session.identity and no longer reads the session map again.
• f93d8ea: the zero-exit helper failure test now checks the typed androidCaptureFailureReason (accessibility-timeout) and errorType, not the message text.

Answers:

• Backoff scope: kept for any failure. A session capture that fails for any reason (request timeout, dead socket, protocol or helper error) is torn down, and the one-shot capture answers. Keying on typed timeout reasons alone would miss the case that matters: a session request timeout on a hung app carries no accessibility-timeout reason. The 40s floor is the fixed one-shot budget (30s) plus the 10s retry floor. The one-shot runs after retire, so its measured duration is not known yet. The PR body now states the any-failure scope.
• Install budget: yes, it is inside the same request. captureAndroidHelperContentAttempt awaits installAndroidSnapshotHelper (30s, snapshot.ts#L78) under the request signal before capturing. It only runs when the helper is missing or outdated, and the per-device cache (snapshot-helper-install.ts#L99-L111) skips it on later attempts. Corrected worst case: about 70s once the helper is installed (10s window + 0.25s delay + forward 5s + start 15s + session request 5s + teardown 2s + one-shot 30s). With a first-run install, one attempt can reach about 87s. That is near the 90s envelope, but the attempt already closes the window, so no re-capture follows. I updated the PR body.
• The cubic threads on concurrent capture and overlapping acquire do not apply. The device execution lock serializes same-device requests (request-binding.ts#L18-L26). Replied and resolved.

Validation on 12757c4:

• pnpm vitest run packages/platform-android/src/__tests__/snapshot*.test.ts: 155 passed.
• pnpm check:affected --run --base upstream/main: all runnable checks passed (385 files / 2641 tests).
• CI: all green, including the Android, Linux, and macOS Smoke Tests, Integration Tests, and Coverage.

Remaining: the 91s hang and the exit-0 failure were not reproduced live, and the live emulator run is from the pre-rebase head.

[thymikee]

The new head 12757c4 fixes what remains of the earlier findings, and I found no new problems. The deadline is now checked again after the delay before a content re-capture (the 9.9s fake-clock test covers it), and the backoff is keyed on the captured session identity.

All 13 checks pass on 12757c4, including the Android snapshot smoke jobs. There are no conflicts.

I read the code and the tests; I did not run them. The live Pixel_9_API_37 run in the PR body was on the pre-rebase head and has no logs. The recheck after the delay was not run on a device; only the unit test covers it. If you can, please post a short log from a device run on this head.

On the open threads: the Copilot thread on the deadline recheck (#3160 (comment)) and the two cubic P2 threads on the deadline recheck and the retire identity (#3160 (comment), #3160 (comment)) are fixed at this head, so you can resolve them. The cubic P2 thread on the stop ordering before failedSessions.set (#3160 (comment)) does not apply. The daemon router serializes same-device commands, so no acquire can overlap the teardown. You can resolve that one too.

Nothing else blocks this PR. It is ready for a maintainer merge decision.