fix: harden Android recording recovery

[thymikee]

Summary

Harden Android recording recovery after daemon restart with a durable device-side manifest.
Preserve session ownership and manifest-known chunks, refuse wrong-session/ambiguous recovery, clean stale manifests, and bound Android recovery probes.
Update recording help/docs to clarify Android chunk guarantees after daemon restart.

Fixes #1130.
Stacked on #1129 because PR #1129 is still open.

Validation

• pnpm format
• pnpm typecheck
• pnpm build
• pnpm vitest run --project provider-integration test/integration/provider-scenarios/android-recording.test.ts
• pnpm test:integration:provider
• pnpm exec vitest run src/cli/parser/tests src/utils/tests/command-schema-guards.test.ts
• pnpm check:unit
• Live Android emulator verification on Pixel 9 Pro XL API 37 / emulator-5554: opened Settings, started recording, scrolled, cleaned daemon while recording was active, then record stop from a fresh daemon recovered the manifest recording and produced a 505,329-byte MP4 with expected overlay-loss warning. Close afterward reported SESSION_NOT_FOUND because daemon state loss removed the original app session and recovered record-only stop released recovery state.

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://callstack.github.io/agent-device/pr-preview/pr-1131/
Built to branch gh-pages at 2026-07-06 15:02 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.5 MB	1.5 MB	+1.2 kB
JS gzip	495.9 kB	495.7 kB	-242 B
npm tarball	591.7 kB	591.7 kB	+14 B
npm unpacked	2.1 MB	2.1 MB	+1.2 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	28.4 ms	28.3 ms	-0.1 ms
CLI --help	52.5 ms	52.3 ms	-0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/record-trace-recording.js	+6.4 kB	+1.5 kB
dist/src/session.js	-1.0 kB	-295 B
dist/src/internal/daemon.js	-850 B	-184 B
dist/src/cli-help.js	+185 B	+63 B
dist/src/command-metadata.js	+94 B	+42 B

[thymikee]

Review finding: the durable-manifest path still has an ownership hole when liveness is uncertain. In readAndroidRecoveryMetadata, a valid manifest whose ps -o pid=,args= -p <pid> probe returns uncertain is skipped, after which recoverMissingAndroidRecording falls through to ownerless findRecoverableAndroidScreenrecord(). If a manifest belongs to another session but the point probe transiently fails, broad ps -A can still find exactly one live screenrecord and record stop --session <wrong-session> will stop/pull another session's recording. That violates #1130's owner-boundary acceptance criterion. A valid durable manifest should block ownerless fallback unless it is proven stale/invalid and cleaned; add coverage for wrong-session manifest + uncertain liveness + broad scan match asserting no kill/pull.

[thymikee]

Review — hardening Android recording recovery (#1131)

Reviewed the incremental delta on top of #1129 (commits 2–7): a durable device-side JSON manifest (/sdcard|/data/local/tmp/agent-device-recording-active.json) now records session ownership, all chunks, and the live pid, so a fresh daemon can validate ownership, refuse wrong-session/ambiguous/uncertain recovery, and pull manifest-known chunks. The stopAndroidRecording refactor is behaviorally equivalent to the pre-refactor version across all {stopError}×{copyError} paths (verified — cleanup runs exactly once, cleanupError never masks copy/stopError), the truncation-warning switch to appendAndroidRecordingWarning correctly concatenates instead of dropping, and the new tests genuinely exercise the live / owner-mismatch / uncertain-block / corrupt-delete / multi-chunk paths. Findings below, most-severe first.

Findings

1. [High] Recovery trusts device-supplied host paths → arbitrary host-file delete + overwrite
record-trace-android-recovery.ts:677 — buildAndroidRecordingFromManifest copies manifest.outPath and manifest.chunks[].path verbatim into the recovered recording, and manifest parsing only checks typeof === 'string' (parseAndroidRecordingChunk:220, readOptionalString for outPath) — no containment check (only remotePath is regex-constrained). On record stop, copyAndroidRecordingChunksWithValidation → copyAndroidRecordingWithValidation runs fs.rmSync(chunk.path, {force:true}) then adb pull … chunk.path, and finalizeAndroidRecordingOutput writes recording.outPath.
Failure: the manifest lives on world-writable /sdcard. A local/malicious writer sets chunks[0].path (or outPath) to e.g. /Users/victim/.ssh/authorized_keys, starts a real screenrecord to a valid agent-device-recording-<n>.mp4 (so liveness passes) with matching sessionName/sessionScope (often the predictable default/cwd scope). After a daemon restart, record stop deletes that host file and overwrites it with attacker-controlled bytes. Benignly, a corrupt manifest redirects the finalized MP4 to the wrong host path. This exposure is new in #1131 — the ownerless path still uses the daemon-resolved recordingBase.outPath. Suggest re-deriving host paths from the request (or validating them against the resolved recording dir) rather than trusting the device.

2. [Med] Rotation persists the manifest too late → crash mid-rotation loses ownership + earlier chunks
record-trace-android-chunks.ts:114 — rotateAndroidRecordingChunk does finishCurrentChunk() (kills the old pid) → startNextChunk() (new pid goes live) → only then persistRecordingState. A daemon crash in that window leaves the on-device manifest pointing at the now-dead old pid.
Failure: on recovery, checkLiveRecoverableAndroidScreenrecord returns stale → the manifest is deleted → findRecoverableAndroidScreenrecord picks up the live new chunk as an ownerless single-chunk recording, dropping session ownership and every earlier rotated chunk — the exact durability the manifest is meant to guarantee. Persisting the new pid before/while killing the old chunk would close the window.

3. [Med] A live recording's durable manifest is deleted on any imperfect read
record-trace-android-recovery.ts:287 (deviceId mismatch), :208 (chunks.length===value.length), :147 (version!==1) — readAndroidRecoveryMetadata treats each of these as invalid/foreign and rms the manifest.
Failure: the same physical device reattaching under a different adb serial (USB↔Wi-Fi / transport-id vs emulator-5554), a single crash-truncated chunk entry, or a future version:2 read by a downgraded daemon destroys the durable state of a still-live recording, degrading it to ownerless recovery (loses session + earlier chunks). This is destructive precisely in the crash/truncation window the feature targets — prefer keeping the manifest and skipping recovery over deleting it.

4. [Med] checkLive pid-reuse false-positive blocks record stop indefinitely
record-trace-android-recovery.ts:342 — sawPid = line.startsWith(metadata.remotePid) returns 'uncertain' whenever the pid is present but not matched as the expected screenrecord+path.
Failure: a finished recording whose pid was reused by an unrelated live process is classified uncertain, so blockAndroidOwnerlessRecoveryForUncertainManifest returns INVALID_ARGS and refuses record stop (and never cleans the manifest) until that process exits.

5. [Low-Med] resetRecoveredRecordingOutput prepares the wrong directory
record-trace-recording.ts:358 — recovery mkdirSync+rmSync the stop-request's freshly-resolved resolvedOut, but the recovered recording actually writes to manifest.outPath/chunks[].path.
Failure: if the manifest's parent dir no longer exists (start ran from a since-removed temp/cwd dir), nothing recreates it and the pull/finalize fails with "failed to copy recording chunk." (Fixing finding #1 by re-deriving host paths would also resolve this.)

6. [Cleanup] Duplicated recovery-decision pipeline and path grammar
record-trace-android-recovery.ts:612 & :569 — selectAndroidRecoveryManifest and blockAndroidOwnerlessRecoveryForUncertainManifest duplicate the same filter→summarize→(0-match owner-mismatch)→(>1 ambiguous) pipeline, diverging only in message text, so a future guard change must edit both. Separately, the remote-dir list + agent-device-recording-<n>.mp4 grammar (:21, :256) is redeclared vs androidRemoteRecordingPaths in record-trace-android.ts:152 — add a writable dir in one place and recovery silently stops matching.

Notes (checked, not blocking)

• The manifests.length > 1 refusal in selectAndroidRecoveryManifest/the uncertain path is largely moot: writeAndroidRecoveryMetadata deletes the other dir's manifest on every write, so the scheme structurally supports at most one recording per device — worth documenting as an explicit limitation.
• Test gap: the deviceId !== deviceId cleanup branch in readAndroidRecoveryMetadata has no coverage (all fixtures use the same device id), so a regression that dropped/inverted that guard wouldn't be caught.

[thymikee]

CI regression after latest head a8633cbc (fix: address android recording recovery review followup): Fallow Code Quality is failing. The workflow is still in progress, so the Fallow log is not downloadable yet (gh run view 28801300165 --job 85405077372 --log reports logs unavailable until completion). Please treat the current green/ready state as stale and run/fix pnpm check:fallow --base origin/main (or the CI FALLOW_BASE once the job completes) before this returns to ready-for-human.

[thymikee]

Fallow log is available now. Exact blocker:

• src/daemon/handlers/record-trace-android-chunks.ts:87 rotateAndroidRecordingChunk exceeds complexity: 12 cyclomatic, 14 cognitive, 52 lines, CRAP 43.1.
• CI base was 126d85ac8a6c67940baf5a356d5eb4c7fcc8edd2; audit scope is 12 changed files.

Focused repro:
pnpm check:fallow --base 126d85ac8a6c67940baf5a356d5eb4c7fcc8edd2

Likely fix: split the rotation decision/cleanup/metadata update pieces into named helpers rather than adding more branching inside rotateAndroidRecordingChunk.