From 31b4629cbc9a695bb68c86fb9e352f5579c11bcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 7 Jul 2026 11:49:37 +0200 Subject: [PATCH 1/2] fix: recover completed Android recording from a pending-only manifest When the daemon crashes in the brief window between writing the pending recovery manifest (before screenrecord starts) and upgrading it to a `current` manifest, the screenrecord process can still finish and leave a complete MP4 on the device. record stop previously discarded it as stale because the pending-only recovery path never checked for an on-device file, unlike the `current` path which already recovers a finished recording. Extend the pending-only path to recover the completed file with the same finished-recording warning, and skip the stop signal when the recovered recording has no tracked pid (a pending chunk never records one, and probing an empty pid is unsafe). --- .../handlers/record-trace-android-recovery.ts | 39 ++++++++++- src/daemon/handlers/record-trace-android.ts | 11 ++++ .../android-recording.test.ts | 66 +++++++++++++++++++ 3 files changed, 113 insertions(+), 3 deletions(-) diff --git a/src/daemon/handlers/record-trace-android-recovery.ts b/src/daemon/handlers/record-trace-android-recovery.ts index bcbdb01af8..fbaa8f8130 100644 --- a/src/daemon/handlers/record-trace-android-recovery.ts +++ b/src/daemon/handlers/record-trace-android-recovery.ts @@ -161,7 +161,14 @@ async function resolvePendingAndroidRecoveryCandidate( const pending = await findLiveAndroidScreenrecordByPath(deviceId, pendingMetadata.remotePath); const adoptedPending = resolveLivePendingScreenrecord(manifest, pending); if (adoptedPending) return adoptedPending; - if (!manifest.current) return pendingOnlyResolution(pending); + if (!manifest.current) { + return await resolvePendingOnlyAndroidRecoveryCandidate( + deviceId, + manifest, + pendingMetadata.remotePath, + pending, + ); + } return await resolveInterruptedRotationCurrent(deviceId, manifest, manifest.current, pending); } @@ -199,8 +206,34 @@ function resolveLivePendingScreenrecord( }); } -function pendingOnlyResolution(pending: AndroidScreenrecordProbe): AndroidRecoveryResolution { - return pending === 'uncertain' ? { kind: 'uncertain' } : { kind: 'stale' }; +async function resolvePendingOnlyAndroidRecoveryCandidate( + deviceId: string, + manifest: AndroidRecordingRecoveryManifest, + pendingRemotePath: string, + pending: AndroidScreenrecordProbe, +): Promise { + if (pending === 'uncertain') { + return { kind: 'uncertain' }; + } + // The pending screenrecord process is gone. If it already produced an on-device file, + // recover it as a finished recording rather than discarding a completed capture — the + // same treatment resolveCurrentAndroidRecoveryCandidate gives a finished `current`. + if (await androidRemoteFileExists(deviceId, pendingRemotePath)) { + return liveAndroidRecoveryCandidate({ + manifest, + current: { + remotePath: pendingRemotePath, + // A pending chunk never recorded a pid — the manifest is written before the + // screenrecord process starts. The process is confirmed gone, so there is + // nothing to signal; the empty pid tells finishCurrentAndroidRecordingChunk to + // skip the stop signal. + remotePid: '', + startedAt: manifest.startedAt, + }, + recoveryWarning: ANDROID_RECOVERY_FINISHED_WARNING, + }); + } + return { kind: 'stale' }; } async function resolveCurrentAndroidRecoveryCandidate( diff --git a/src/daemon/handlers/record-trace-android.ts b/src/daemon/handlers/record-trace-android.ts index 5ba2574dfb..e496caeac7 100644 --- a/src/daemon/handlers/record-trace-android.ts +++ b/src/daemon/handlers/record-trace-android.ts @@ -542,6 +542,17 @@ async function finishCurrentAndroidRecordingChunk(params: { remotePid = recording.remotePid, waitForRemoteFileStability = true, } = params; + if (!remotePid) { + // A recovered finished recording with no tracked process (a pending chunk whose + // screenrecord already exited): there is nothing to signal, and the on-device file + // is already complete. Skip the kill entirely — probing/signalling an empty pid is + // unsafe (`isAndroidProcessRunning('')` can report a false positive). + appendAndroidRecordingWarning(recording, resolveAndroidScreenrecordLimitWarning(recording)); + if (waitForRemoteFileStability) { + await waitForAndroidRemoteFileStability(device.id, remotePath); + } + return undefined; + } const wasRunningBeforeStop = await isAndroidProcessRunning(device.id, remotePid); if (!wasRunningBeforeStop) { appendAndroidRecordingWarning(recording, resolveAndroidScreenrecordLimitWarning(recording)); diff --git a/test/integration/provider-scenarios/android-recording.test.ts b/test/integration/provider-scenarios/android-recording.test.ts index 6c4b0152b4..16fc06eecc 100644 --- a/test/integration/provider-scenarios/android-recording.test.ts +++ b/test/integration/provider-scenarios/android-recording.test.ts @@ -229,6 +229,13 @@ test('Provider-backed integration Android record stop recovers pending manifest ); }); +test('Provider-backed integration Android record stop recovers finished pending manifest after process exit', async () => { + await withProviderScenarioTempDir( + 'agent-device-provider-scenario-android-record-pending-finished-', + runAndroidPendingFinishedManifestRecoveryScenario, + ); +}); + test('Provider-backed integration Android record stop recovers rotating manifest after daemon state loss', async () => { await withProviderScenarioTempDir( 'agent-device-provider-scenario-android-record-rotating-recovery-', @@ -637,6 +644,65 @@ async function runAndroidPendingManifestRecoveryScenario(tmpDir: string): Promis } } +async function runAndroidPendingFinishedManifestRecoveryScenario(tmpDir: string): Promise { + const adbCalls: string[][] = []; + const pullCalls: PullCall[] = []; + const remotePath = '/sdcard/agent-device-recording-624000001.mp4'; + const recordingPath = path.join(tmpDir, 'pending-finished-recovered.mp4'); + const manifest = buildAndroidRecordingManifest({ + outPath: recordingPath, + remotePath, + sessionName: 'default', + status: 'pending', + }); + const daemon = await createProviderScenarioHarness({ + androidAdbProvider: () => ({ + exec: async (args) => { + adbCalls.push([...args]); + const command = args.join(' '); + if (command === 'shell cat /sdcard/agent-device-recording-active.json') { + return { stdout: JSON.stringify(manifest), stderr: '', exitCode: 0 }; + } + if (command === 'shell cat /data/local/tmp/agent-device-recording-active.json') { + return { stdout: '', stderr: '', exitCode: 1 }; + } + // The pending screenrecord process already exited: the full process scan finds no + // match, but the on-device file still exists (default stat returns a non-zero size). + if (command === 'shell ps -A -o pid=,args=') { + return { stdout: '', stderr: '', exitCode: 0 }; + } + return androidAdbResult(args); + }, + pull: async (from, to) => { + pullCalls.push({ remotePath: from, localPath: to }); + writePlayableMp4(to); + return { stdout: '', stderr: '', exitCode: 0 }; + }, + }), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); + + try { + const recordStop = await stopAndroidRecording(daemon, recordingPath); + const data = assertRpcOk<{ recording?: unknown; outPath?: unknown; warning?: unknown }>( + recordStop, + ); + assert.equal(data.recording, 'stopped'); + assert.equal(data.outPath, recordingPath); + assert.match(String(data.warning), /no longer running/); + // A pending manifest never recorded a pid and the process is confirmed gone, so stop + // sends no signal — it just pulls the completed file instead of discarding it. + assert.equal( + adbCalls.some((args) => args.join(' ').startsWith('shell kill -2')), + false, + ); + assert.deepEqual(pullCalls, [{ remotePath, localPath: recordingPath }]); + assert.equal(fs.existsSync(recordingPath), true); + } finally { + await daemon.close(); + } +} + async function runAndroidRotatingManifestRecoveryScenario(tmpDir: string): Promise { const adbCalls: string[][] = []; const pullCalls: PullCall[] = []; From 8c61f94e53900d839a793504586e6e8d759e559c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Tue, 7 Jul 2026 11:49:37 +0200 Subject: [PATCH 2/2] fix: treat JSON arrays as invalid Android recovery manifests isRecord accepted arrays (typeof [] === 'object'), so a stray `[]` recovery manifest was classified as blocked rather than deleted, wedging every subsequent record stop. Reject arrays and null so a non-object manifest is cleaned up like other malformed metadata. --- src/daemon/handlers/record-trace-android-recovery-manifest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/daemon/handlers/record-trace-android-recovery-manifest.ts b/src/daemon/handlers/record-trace-android-recovery-manifest.ts index 6372f23315..7e0de268ae 100644 --- a/src/daemon/handlers/record-trace-android-recovery-manifest.ts +++ b/src/daemon/handlers/record-trace-android-recovery-manifest.ts @@ -186,7 +186,7 @@ function parseJsonObject(value: string): Record | undefined { } function isRecord(value: unknown): value is Record { - return Boolean(value) && typeof value === 'object'; + return typeof value === 'object' && value !== null && !Array.isArray(value); } function readAndroidRecoveryManifestRequired(