From da2a0aa7d8aa391fd08cabd74dc3b63ceeb0ce7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 6 Jul 2026 12:31:04 +0200 Subject: [PATCH 1/3] fix: recover android recordings after daemon restart --- .../handlers/record-trace-android-recovery.ts | 316 ++++++++++++++++++ src/daemon/handlers/record-trace-android.ts | 15 + .../record-trace-recording-backends.ts | 8 + src/daemon/handlers/record-trace-recording.ts | 61 +++- .../android-recording.test.ts | 74 ++++ 5 files changed, 470 insertions(+), 4 deletions(-) create mode 100644 src/daemon/handlers/record-trace-android-recovery.ts diff --git a/src/daemon/handlers/record-trace-android-recovery.ts b/src/daemon/handlers/record-trace-android-recovery.ts new file mode 100644 index 000000000..b419dc010 --- /dev/null +++ b/src/daemon/handlers/record-trace-android-recovery.ts @@ -0,0 +1,316 @@ +import path from 'node:path'; +import { androidDeviceForSerial, runAndroidAdb } from '../../platforms/android/adb.ts'; +import type { + AndroidAdbExecutorOptions, + AndroidAdbExecutorResult, +} from '../../platforms/android/adb-executor.ts'; +import { shellQuote } from '../../utils/shell-quote.ts'; +import { emitDiagnostic } from '../../utils/diagnostics.ts'; +import type { DaemonResponse, SessionState } from '../types.ts'; +import { errorResponse } from './response.ts'; + +const ANDROID_RECOVERY_WARNING = + 'Recovered Android recording after daemon recording state was missing; gesture overlays and earlier rotated chunks may be unavailable.'; +const ANDROID_RECOVERY_METADATA_FILE = 'agent-device-recording-active.json'; +const ANDROID_RECOVERY_PROBE_TIMEOUT_MS = 5_000; +const ANDROID_RECOVERY_METADATA_DIRS = ['/sdcard', '/data/local/tmp'] as const; + +type AndroidDevice = SessionState['device']; +type AndroidRecording = Extract, { platform: 'android' }>; +type AndroidRecordingBase = Pick< + AndroidRecording, + | 'outPath' + | 'clientOutPath' + | 'telemetryPath' + | 'startedAt' + | 'maxSize' + | 'exportQuality' + | 'showTouches' + | 'gestureEvents' +>; + +type AndroidRecordingRecoveryMetadata = { + remotePath: string; + remotePid: string; + startedAt: number; +}; + +async function runAndroidRecoveryAdb( + deviceId: string, + args: string[], + options?: AndroidAdbExecutorOptions, +): Promise { + return await runAndroidAdb(androidDeviceForSerial(deviceId), args, options); +} + +function parseRecoverableAndroidScreenrecord( + line: string, +): AndroidRecordingRecoveryMetadata | undefined { + const match = line + .trim() + .match( + /^(\d+)\s+.*\bscreenrecord\b.*(\/(?:sdcard|data\/local\/tmp)\/agent-device-recording-(\d+)\.mp4)(?:\s|$)/, + ); + if (!match) { + return undefined; + } + const [, remotePid, remotePath, timestamp] = match; + if (!remotePid || !remotePath) { + return undefined; + } + const startedAt = Number(timestamp); + return { + remotePid, + remotePath, + startedAt: Number.isFinite(startedAt) ? startedAt : Date.now(), + }; +} + +function parseAndroidRecoveryMetadata(value: string): AndroidRecordingRecoveryMetadata | undefined { + let parsed: unknown; + try { + parsed = JSON.parse(value); + } catch { + return undefined; + } + if (!parsed || typeof parsed !== 'object') { + return undefined; + } + const metadata = parsed as Partial; + if ( + typeof metadata.remotePid !== 'string' || + !/^\d+$/.test(metadata.remotePid) || + typeof metadata.remotePath !== 'string' || + !isAndroidAgentRecordingPath(metadata.remotePath) + ) { + return undefined; + } + return { + remotePid: metadata.remotePid, + remotePath: metadata.remotePath, + startedAt: + typeof metadata.startedAt === 'number' && Number.isFinite(metadata.startedAt) + ? metadata.startedAt + : Date.now(), + }; +} + +function isAndroidAgentRecordingPath(remotePath: string): boolean { + return /^\/(?:sdcard|data\/local\/tmp)\/agent-device-recording-\d+\.mp4$/.test(remotePath); +} + +function androidRecoveryMetadataPathForRemotePath(remotePath: string): string { + return `${path.posix.dirname(remotePath)}/${ANDROID_RECOVERY_METADATA_FILE}`; +} + +function androidRecoveryMetadataPaths(): string[] { + return ANDROID_RECOVERY_METADATA_DIRS.map((dir) => `${dir}/${ANDROID_RECOVERY_METADATA_FILE}`); +} + +async function readAndroidRecoveryMetadata( + deviceId: string, +): Promise { + for (const metadataPath of androidRecoveryMetadataPaths()) { + const result = await runAndroidRecoveryAdb(deviceId, ['shell', 'cat', metadataPath], { + allowFailure: true, + timeoutMs: ANDROID_RECOVERY_PROBE_TIMEOUT_MS, + }); + if (result.exitCode !== 0) { + continue; + } + const metadata = parseAndroidRecoveryMetadata(result.stdout); + if (!metadata) { + continue; + } + if (await isLiveRecoverableAndroidScreenrecord(deviceId, metadata)) { + return metadata; + } + await cleanupAndroidRecoveryMetadataPath({ + deviceId, + metadataPath, + phase: 'record_stop_android_recovery_metadata_stale_cleanup_failed', + }); + } + return undefined; +} + +async function isLiveRecoverableAndroidScreenrecord( + deviceId: string, + metadata: AndroidRecordingRecoveryMetadata, +): Promise { + const result = await runAndroidRecoveryAdb( + deviceId, + ['shell', 'ps', '-o', 'pid=,args=', '-p', metadata.remotePid], + { + allowFailure: true, + timeoutMs: ANDROID_RECOVERY_PROBE_TIMEOUT_MS, + }, + ); + if (result.exitCode !== 0) { + return false; + } + return result.stdout + .split(/\r?\n/) + .map(parseRecoverableAndroidScreenrecord) + .some( + (candidate) => + candidate?.remotePid === metadata.remotePid && candidate.remotePath === metadata.remotePath, + ); +} + +async function findRecoverableAndroidScreenrecord( + deviceId: string, +): Promise { + const result = await runAndroidRecoveryAdb(deviceId, ['shell', 'ps', '-A', '-o', 'pid=,args='], { + allowFailure: true, + timeoutMs: ANDROID_RECOVERY_PROBE_TIMEOUT_MS, + }); + if (result.exitCode !== 0) { + emitDiagnostic({ + level: 'debug', + phase: 'record_stop_android_recovery_ps_failed', + data: { + deviceId, + exitCode: result.exitCode, + stdout: result.stdout.trim(), + stderr: result.stderr.trim(), + }, + }); + return undefined; + } + + const matches = result.stdout + .split(/\r?\n/) + .map(parseRecoverableAndroidScreenrecord) + .filter((match): match is NonNullable => match !== undefined) + .sort((a, b) => b.startedAt - a.startedAt); + if (matches.length === 0) { + return undefined; + } + if (matches.length > 1) { + return errorResponse( + 'INVALID_ARGS', + 'multiple active Android screenrecord processes match agent-device recordings; cannot safely recover missing recording state', + { processes: matches.map(({ remotePid, remotePath }) => ({ remotePid, remotePath })) }, + ); + } + return matches[0]; +} + +export async function writeAndroidRecoveryMetadata( + deviceId: string, + metadata: AndroidRecordingRecoveryMetadata, +): Promise { + const metadataPath = androidRecoveryMetadataPathForRemotePath(metadata.remotePath); + const payload = JSON.stringify(metadata); + const result = await runAndroidRecoveryAdb( + deviceId, + ['shell', `printf %s ${shellQuote(payload)} > ${shellQuote(metadataPath)}`], + { + allowFailure: true, + timeoutMs: ANDROID_RECOVERY_PROBE_TIMEOUT_MS, + }, + ); + if (result.exitCode !== 0) { + emitDiagnostic({ + level: 'warn', + phase: 'record_start_android_recovery_metadata_failed', + data: { + deviceId, + metadataPath, + exitCode: result.exitCode, + stdout: result.stdout.trim(), + stderr: result.stderr.trim(), + }, + }); + return; + } + + for (const staleMetadataPath of androidRecoveryMetadataPaths()) { + if (staleMetadataPath !== metadataPath) { + await cleanupAndroidRecoveryMetadataPath({ + deviceId, + metadataPath: staleMetadataPath, + phase: 'record_start_android_recovery_metadata_stale_cleanup_failed', + }); + } + } +} + +export async function cleanupAndroidRecoveryMetadata(deviceId: string): Promise { + for (const metadataPath of androidRecoveryMetadataPaths()) { + await cleanupAndroidRecoveryMetadataPath({ + deviceId, + metadataPath, + phase: 'record_stop_android_recovery_metadata_cleanup_failed', + }); + } +} + +async function cleanupAndroidRecoveryMetadataPath(params: { + deviceId: string; + metadataPath: string; + phase: string; +}): Promise { + const { deviceId, metadataPath, phase } = params; + const result = await runAndroidRecoveryAdb(deviceId, ['shell', 'rm', '-f', metadataPath], { + allowFailure: true, + timeoutMs: ANDROID_RECOVERY_PROBE_TIMEOUT_MS, + }); + if (result.exitCode !== 0) { + emitDiagnostic({ + level: 'warn', + phase, + data: { + deviceId, + metadataPath, + exitCode: result.exitCode, + stdout: result.stdout.trim(), + stderr: result.stderr.trim(), + }, + }); + } +} + +export async function recoverMissingAndroidRecording(params: { + device: AndroidDevice; + recordingBase: AndroidRecordingBase; +}): Promise { + const { device, recordingBase } = params; + const recovered = + (await readAndroidRecoveryMetadata(device.id)) ?? + (await findRecoverableAndroidScreenrecord(device.id)); + if (!recovered) { + return null; + } + if ('ok' in recovered) { + return recovered; + } + + emitDiagnostic({ + level: 'warn', + phase: 'record_stop_android_recovered_missing_state', + data: { + deviceId: device.id, + remotePath: recovered.remotePath, + remotePid: recovered.remotePid, + outPath: recordingBase.outPath, + }, + }); + + return { + platform: 'android', + remotePath: recovered.remotePath, + remotePid: recovered.remotePid, + chunks: [ + { + index: 1, + path: recordingBase.outPath, + remotePath: recovered.remotePath, + }, + ], + ...recordingBase, + startedAt: recovered.startedAt, + warning: ANDROID_RECOVERY_WARNING, + }; +} diff --git a/src/daemon/handlers/record-trace-android.ts b/src/daemon/handlers/record-trace-android.ts index 99d23f75e..d46ed95ea 100644 --- a/src/daemon/handlers/record-trace-android.ts +++ b/src/daemon/handlers/record-trace-android.ts @@ -20,6 +20,10 @@ import { DEFAULT_RECORDING_EXPORT_QUALITY, type RecordingExportQuality, } from '../../core/recording-export-quality.ts'; +import { + cleanupAndroidRecoveryMetadata, + writeAndroidRecoveryMetadata, +} from './record-trace-android-recovery.ts'; type AndroidRecordingSize = { width: number; height: number }; @@ -343,6 +347,11 @@ export async function startAndroidRecording(params: { ...recordingBase, startedAt: chunk.startedAt, }; + await writeAndroidRecoveryMetadata(device.id, { + remotePath: recording.remotePath, + remotePid: recording.remotePid, + startedAt: recording.startedAt, + }); scheduleAndroidRecordingRotation({ recording, finishCurrentChunk: async () => @@ -365,6 +374,11 @@ export async function startAndroidRecording(params: { : nextChunk.error.error.message, ); } + await writeAndroidRecoveryMetadata(device.id, { + remotePath: nextChunk.remotePath, + remotePid: nextChunk.remotePid, + startedAt: nextChunk.startedAt, + }); return nextChunk; }, }); @@ -527,6 +541,7 @@ export async function stopAndroidRecording(params: { cleanupError = `failed to clean up remote recording: ${formatRecordTraceExecFailure(rmResult, 'adb shell rm')}`; } } + await cleanupAndroidRecoveryMetadata(device.id); } } diff --git a/src/daemon/handlers/record-trace-recording-backends.ts b/src/daemon/handlers/record-trace-recording-backends.ts index 2308198cd..d4d8e2419 100644 --- a/src/daemon/handlers/record-trace-recording-backends.ts +++ b/src/daemon/handlers/record-trace-recording-backends.ts @@ -12,6 +12,7 @@ import { import { resolveWebProvider } from '../../platforms/web/provider.ts'; import { errorResponse } from './response.ts'; import { startAndroidRecording, stopAndroidRecording } from './record-trace-android.ts'; +import { recoverMissingAndroidRecording } from './record-trace-android-recovery.ts'; import { normalizeAppBundleId, startIosDeviceRecording, @@ -77,6 +78,8 @@ type RecordingStopContext

= { stopRequestedAt: number; }; +type MissingRecordingStopContext = Omit; + // A backend is parameterized by the recording tag it owns, so its `stop` receives an // already-narrowed recording — no `recording as Extract` casts. // `start` stays wide because the device platform does not map 1:1 to a recording tag @@ -86,6 +89,9 @@ export type RecordingBackend

= resolveOutputPath: (context: RecordingOutputPathContext) => string; start: (context: RecordingStartContext) => Promise; stop: (context: RecordingStopContext

) => Promise; + recoverMissingStop?: ( + context: MissingRecordingStopContext, + ) => Promise; cleanupRecordOnlySession?: (session: SessionState) => Promise; }; @@ -272,6 +278,8 @@ const androidRecordingBackend: RecordingBackend<'android'> = { resolveOutputPath: resolveNativeRecordingOutputPath, start: async ({ device, recordingBase }) => await startAndroidRecording({ device, recordingBase }), + recoverMissingStop: async ({ device, recordingBase }) => + await recoverMissingAndroidRecording({ device, recordingBase }), stop: async ({ deps, device, recording, stopRequestedAt }) => await stopAndroidRecording({ deps, diff --git a/src/daemon/handlers/record-trace-recording.ts b/src/daemon/handlers/record-trace-recording.ts index 48bb9fed2..59c999f30 100644 --- a/src/daemon/handlers/record-trace-recording.ts +++ b/src/daemon/handlers/record-trace-recording.ts @@ -169,18 +169,26 @@ async function startRecording(params: { async function stopRecording(params: { req: DaemonRequest; + sessionStore: SessionStore; activeSession: SessionState; device: SessionState['device']; logPath?: string; deps: RecordTraceDeps; }): Promise { - const { req, activeSession, device, logPath, deps } = params; + const { req, sessionStore, activeSession, device, logPath, deps } = params; - if (!activeSession.recording) { + let recording = activeSession.recording; + if (!recording) { + const recovered = await recoverMissingRecordingState(); + if (recovered && 'ok' in recovered) { + return recovered; + } + recording = recovered ?? undefined; + } + if (!recording) { return errorResponse('INVALID_ARGS', 'no active recording'); } - const recording = activeSession.recording; const stopRequestedAt = Date.now(); const invalidatedReason = recording.invalidatedReason; activeSession.recording = undefined; @@ -204,6 +212,51 @@ async function stopRecording(params: { } return buildRecordStopResponse(recording); + + async function recoverMissingRecordingState(): Promise< + DaemonResponse | NonNullable | null + > { + if (hasActiveRecordingSession(sessionStore)) { + return null; + } + + const backend = resolveRecordingBackendForDevice(device); + if (!backend.recoverMissingStop) { + return null; + } + + const outPath = backend.resolveOutputPath({ req }); + const resolvedOut = SessionStore.expandHome(outPath, req.meta?.cwd); + const recordingBase = buildRecordingBase(req, resolvedOut); + + const recovered = await backend.recoverMissingStop({ + req, + activeSession, + sessionStore, + device, + logPath, + deps, + recordingBase, + resolvedOut, + }); + if (!recovered) { + return null; + } + if (!('ok' in recovered)) { + fs.mkdirSync(path.dirname(resolvedOut), { recursive: true }); + fs.rmSync(resolvedOut, { force: true }); + } + return recovered; + } +} + +function hasActiveRecordingSession(sessionStore: SessionStore): boolean { + for (const session of sessionStore.values()) { + if (session.recording) { + return true; + } + } + return false; } function buildRecordStopResponse( @@ -330,7 +383,7 @@ export async function handleRecordCommand(params: { return startRecording({ req, sessionName, sessionStore, activeSession, device, logPath, deps }); } - const response = await stopRecording({ req, activeSession, device, logPath, deps }); + const response = await stopRecording({ req, sessionStore, activeSession, device, logPath, deps }); if (!response.ok) { await releaseRecordOnlySession(sessionStore, sessionName, activeSession); return response; diff --git a/test/integration/provider-scenarios/android-recording.test.ts b/test/integration/provider-scenarios/android-recording.test.ts index 6d6a2574f..5cce9dc2c 100644 --- a/test/integration/provider-scenarios/android-recording.test.ts +++ b/test/integration/provider-scenarios/android-recording.test.ts @@ -91,6 +91,80 @@ test('Provider-backed integration Android recording flow uses scripted ADB provi ); }); +test('Provider-backed integration Android record stop recovers missing daemon recording state', async () => { + await withProviderScenarioTempDir( + 'agent-device-provider-scenario-android-record-recovery-', + async (tmpDir) => { + const adbCalls: string[][] = []; + const pullCalls: Array<{ remotePath: string; localPath: string }> = []; + const remotePath = '/sdcard/agent-device-recording-123456789.mp4'; + const adbProvider: AndroidAdbProvider = { + exec: async (args) => { + adbCalls.push([...args]); + if (args.join(' ') === 'shell ps -A -o pid=,args=') { + return { + stdout: `4321 screenrecord --bit-rate 8000000 ${remotePath}\n`, + stderr: '', + exitCode: 0, + }; + } + return androidAdbResult(args); + }, + pull: async (from, to) => { + pullCalls.push({ remotePath: from, localPath: to }); + fs.writeFileSync(to, likelyPlayableMp4Container()); + return { stdout: '', stderr: '', exitCode: 0 }; + }, + }; + const daemon = await createProviderScenarioHarness({ + androidAdbProvider: () => adbProvider, + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); + + const previousPath = process.env.PATH; + const previousSwiftCacheDir = process.env.AGENT_DEVICE_SWIFT_CACHE_DIR; + process.env.PATH = tmpDir; + process.env.AGENT_DEVICE_SWIFT_CACHE_DIR = path.join(tmpDir, 'swift-cache'); + + try { + const recordStop = await daemon.callCommand( + 'record', + ['stop'], + { + platform: 'android', + serial: PROVIDER_SCENARIO_ANDROID.id, + }, + { meta: { cwd: tmpDir } }, + ); + const data = assertRpcOk<{ + recording?: unknown; + outPath?: unknown; + warning?: unknown; + }>(recordStop); + assert.equal(data.recording, 'stopped'); + const outPath = data.outPath; + assert.equal(typeof outPath, 'string'); + if (typeof outPath !== 'string') { + throw new Error(`expected string outPath, got ${String(outPath)}`); + } + assert.match(outPath, /\/recording-\d+\.mp4$/); + assert.match(String(data.warning), /Recovered Android recording/); + assert.equal(fs.existsSync(outPath), true); + + assertCommandCall(adbCalls, ['shell', 'ps', '-A', '-o', 'pid=,args=']); + assertCommandCall(adbCalls, ['shell', 'kill', '-2', '4321']); + assert.equal(pullCalls.length, 1); + assert.deepEqual(pullCalls[0], { remotePath, localPath: outPath }); + assertCommandCall(adbCalls, ['shell', 'rm', '-f', remotePath]); + } finally { + await daemon.close(); + restoreEnv('PATH', previousPath); + restoreEnv('AGENT_DEVICE_SWIFT_CACHE_DIR', previousSwiftCacheDir); + } + }, + ); +}); + test('Provider-backed integration Android record start without a session scopes default-device providers', async () => { await withMockedAdb( 'agent-device-provider-scenario-android-sessionless-record-', From 801ac2f5d56b95081327ddae7278612cda642049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 6 Jul 2026 12:57:19 +0200 Subject: [PATCH 2/3] refactor: reduce recording recovery complexity --- .../handlers/record-trace-android-recovery.ts | 47 +- src/daemon/handlers/record-trace-android.ts | 183 +++++--- src/daemon/handlers/record-trace-recording.ts | 338 +++++++++----- .../android-recording.test.ts | 419 +++++++++++------- 4 files changed, 635 insertions(+), 352 deletions(-) diff --git a/src/daemon/handlers/record-trace-android-recovery.ts b/src/daemon/handlers/record-trace-android-recovery.ts index b419dc010..6e95f1ccc 100644 --- a/src/daemon/handlers/record-trace-android-recovery.ts +++ b/src/daemon/handlers/record-trace-android-recovery.ts @@ -67,6 +67,23 @@ function parseRecoverableAndroidScreenrecord( } function parseAndroidRecoveryMetadata(value: string): AndroidRecordingRecoveryMetadata | undefined { + const metadata = parseAndroidRecoveryMetadataObject(value); + if (!metadata) { + return undefined; + } + const remotePid = parseAndroidRecoveryRemotePid(metadata.remotePid); + const remotePath = parseAndroidRecoveryRemotePath(metadata.remotePath); + if (!remotePid || !remotePath) { + return undefined; + } + return { + remotePid, + remotePath, + startedAt: parseAndroidRecoveryStartedAt(metadata.startedAt), + }; +} + +function parseAndroidRecoveryMetadataObject(value: string): Record | undefined { let parsed: unknown; try { parsed = JSON.parse(value); @@ -76,23 +93,19 @@ function parseAndroidRecoveryMetadata(value: string): AndroidRecordingRecoveryMe if (!parsed || typeof parsed !== 'object') { return undefined; } - const metadata = parsed as Partial; - if ( - typeof metadata.remotePid !== 'string' || - !/^\d+$/.test(metadata.remotePid) || - typeof metadata.remotePath !== 'string' || - !isAndroidAgentRecordingPath(metadata.remotePath) - ) { - return undefined; - } - return { - remotePid: metadata.remotePid, - remotePath: metadata.remotePath, - startedAt: - typeof metadata.startedAt === 'number' && Number.isFinite(metadata.startedAt) - ? metadata.startedAt - : Date.now(), - }; + return parsed as Record; +} + +function parseAndroidRecoveryRemotePid(value: unknown): string | undefined { + return typeof value === 'string' && /^\d+$/.test(value) ? value : undefined; +} + +function parseAndroidRecoveryRemotePath(value: unknown): string | undefined { + return typeof value === 'string' && isAndroidAgentRecordingPath(value) ? value : undefined; +} + +function parseAndroidRecoveryStartedAt(value: unknown): number { + return typeof value === 'number' && Number.isFinite(value) ? value : Date.now(); } function isAndroidAgentRecordingPath(remotePath: string): boolean { diff --git a/src/daemon/handlers/record-trace-android.ts b/src/daemon/handlers/record-trace-android.ts index d46ed95ea..01563b4c1 100644 --- a/src/daemon/handlers/record-trace-android.ts +++ b/src/daemon/handlers/record-trace-android.ts @@ -53,6 +53,11 @@ type AndroidRecordingBase = Pick< | 'showTouches' | 'gestureEvents' >; +type AndroidRecordingChunkStart = { + remotePath: string; + remotePid: string; + startedAt: number; +}; async function runAndroidRecordingAdb( deviceId: string, @@ -253,9 +258,7 @@ async function startAndroidScreenrecordChunk(params: { recordingSize: AndroidRecordingSize | undefined; quality: RecordingExportQuality; preferredRemoteDir?: string; -}): Promise< - { remotePath: string; remotePid: string; startedAt: number } | { error: DaemonResponse } -> { +}): Promise { const { device, recordingSize, quality, preferredRemoteDir } = params; let lastStartError = 'failed to start recording: Android screenrecord did not begin producing frames'; @@ -333,7 +336,23 @@ export async function startAndroidRecording(params: { return chunk.error; } - const recording: AndroidRecording = { + const recording = buildAndroidRecording({ recordingBase, chunk }); + await writeAndroidRecoveryMetadataForChunk(device.id, chunk); + scheduleAndroidRecordingChunks({ + device, + recording, + recordingSize, + quality, + }); + return recording; +} + +function buildAndroidRecording(params: { + recordingBase: AndroidRecordingBase; + chunk: AndroidRecordingChunkStart; +}): AndroidRecording { + const { recordingBase, chunk } = params; + return { platform: 'android', remotePath: chunk.remotePath, remotePid: chunk.remotePid, @@ -347,11 +366,26 @@ export async function startAndroidRecording(params: { ...recordingBase, startedAt: chunk.startedAt, }; - await writeAndroidRecoveryMetadata(device.id, { - remotePath: recording.remotePath, - remotePid: recording.remotePid, - startedAt: recording.startedAt, +} + +async function writeAndroidRecoveryMetadataForChunk( + deviceId: string, + chunk: AndroidRecordingChunkStart, +): Promise { + await writeAndroidRecoveryMetadata(deviceId, { + remotePath: chunk.remotePath, + remotePid: chunk.remotePid, + startedAt: chunk.startedAt, }); +} + +function scheduleAndroidRecordingChunks(params: { + device: AndroidDevice; + recording: AndroidRecording; + recordingSize: AndroidRecordingSize | undefined; + quality: RecordingExportQuality; +}): void { + const { device, recording, recordingSize, quality } = params; scheduleAndroidRecordingRotation({ recording, finishCurrentChunk: async () => @@ -374,15 +408,10 @@ export async function startAndroidRecording(params: { : nextChunk.error.error.message, ); } - await writeAndroidRecoveryMetadata(device.id, { - remotePath: nextChunk.remotePath, - remotePid: nextChunk.remotePid, - startedAt: nextChunk.startedAt, - }); + await writeAndroidRecoveryMetadataForChunk(device.id, nextChunk); return nextChunk; }, }); - return recording; } async function finishCurrentAndroidRecordingChunk(params: { @@ -474,36 +503,29 @@ export async function stopAndroidRecording(params: { }, }); recording.stopping = true; - if (recording.rotationTimer) { - clearTimeout(recording.rotationTimer); - recording.rotationTimer = undefined; - } - await recording.rotationPromise; + await finishPendingAndroidRecordingRotation(recording); const stopError = await finishCurrentAndroidRecordingChunk({ device, recording }); if (recording.rotationFailedReason && !stopError) { recording.warning ??= `Android recording chunk rotation failed: ${recording.rotationFailedReason}`; } - let cleanupError: string | undefined; - if (!stopError) { - const copyError = await copyAndroidRecordingChunksWithValidation({ - deps, - deviceId: device.id, - chunks: ensureAndroidRecordingChunks(recording), - }); - if (copyError) { - await cleanupRemoteRecording(); - return errorResponse( - 'COMMAND_FAILED', - formatAndroidStopFailure(copyError, recording, stopRequestedAt), - ); - } + const copyError = + stopError === undefined + ? await copyAndFinalizeAndroidRecording({ deps, device, recording }) + : undefined; + const cleanupError = await cleanupRemoteAndroidRecordingChunks({ + deviceId: device.id, + recording, + recordCleanupError: stopError === undefined, + }); - await finalizeAndroidRecordingOutput({ recording, deps }); + if (copyError) { + return errorResponse( + 'COMMAND_FAILED', + formatAndroidStopFailure(copyError, recording, stopRequestedAt), + ); } - await cleanupRemoteRecording(); - if (stopError) { return errorResponse( 'COMMAND_FAILED', @@ -516,33 +538,74 @@ export async function stopAndroidRecording(params: { } return null; +} - async function cleanupRemoteRecording(): Promise { - for (const chunk of ensureAndroidRecordingChunks(recording)) { - const rmResult = await runAndroidRecordingAdb( - device.id, - ['shell', 'rm', '-f', chunk.remotePath], - { - allowFailure: true, - }, - ); - emitDiagnostic({ - level: 'debug', - phase: 'record_stop_android_cleanup', - data: { - deviceId: device.id, - remotePath: chunk.remotePath, - exitCode: rmResult.exitCode, - stdout: rmResult.stdout.trim(), - stderr: rmResult.stderr.trim(), - }, - }); - if (rmResult.exitCode !== 0 && !stopError) { - cleanupError = `failed to clean up remote recording: ${formatRecordTraceExecFailure(rmResult, 'adb shell rm')}`; - } +async function finishPendingAndroidRecordingRotation(recording: AndroidRecording): Promise { + if (recording.rotationTimer) { + clearTimeout(recording.rotationTimer); + recording.rotationTimer = undefined; + } + await recording.rotationPromise; +} + +async function copyAndFinalizeAndroidRecording(params: { + deps: RecordTraceDeps; + device: AndroidDevice; + recording: AndroidRecording; +}): Promise { + const { deps, device, recording } = params; + const copyError = await copyAndroidRecordingChunksWithValidation({ + deps, + deviceId: device.id, + chunks: ensureAndroidRecordingChunks(recording), + }); + if (copyError) { + return copyError; + } + + await finalizeAndroidRecordingOutput({ recording, deps }); + return undefined; +} + +async function cleanupRemoteAndroidRecordingChunks(params: { + deviceId: string; + recording: AndroidRecording; + recordCleanupError: boolean; +}): Promise { + const { deviceId, recording, recordCleanupError } = params; + let cleanupError: string | undefined; + for (const chunk of ensureAndroidRecordingChunks(recording)) { + const chunkCleanupError = await cleanupRemoteAndroidRecordingChunk(deviceId, chunk.remotePath); + if (chunkCleanupError && recordCleanupError) { + cleanupError = chunkCleanupError; } - await cleanupAndroidRecoveryMetadata(device.id); } + await cleanupAndroidRecoveryMetadata(deviceId); + return cleanupError; +} + +async function cleanupRemoteAndroidRecordingChunk( + deviceId: string, + remotePath: string, +): Promise { + const rmResult = await runAndroidRecordingAdb(deviceId, ['shell', 'rm', '-f', remotePath], { + allowFailure: true, + }); + emitDiagnostic({ + level: 'debug', + phase: 'record_stop_android_cleanup', + data: { + deviceId, + remotePath, + exitCode: rmResult.exitCode, + stdout: rmResult.stdout.trim(), + stderr: rmResult.stderr.trim(), + }, + }); + if (rmResult.exitCode !== 0) { + return `failed to clean up remote recording: ${formatRecordTraceExecFailure(rmResult, 'adb shell rm')}`; + } + return undefined; } function formatAndroidStopFailure( diff --git a/src/daemon/handlers/record-trace-recording.ts b/src/daemon/handlers/record-trace-recording.ts index 59c999f30..74038716a 100644 --- a/src/daemon/handlers/record-trace-recording.ts +++ b/src/daemon/handlers/record-trace-recording.ts @@ -37,6 +37,37 @@ const IOS_SIMULATOR_RECORDING_TAIL_SETTLE_MS = 350; export type { RecordTraceDeps, RecordingBase } from './record-trace-types.ts'; +type StartRecordingParams = { + req: DaemonRequest; + sessionName: string; + sessionStore: SessionStore; + activeSession: SessionState; + device: SessionState['device']; + logPath?: string; + deps: RecordTraceDeps; +}; + +type StopRecordingParams = { + req: DaemonRequest; + sessionStore: SessionStore; + activeSession: SessionState; + device: SessionState['device']; + logPath?: string; + deps: RecordTraceDeps; +}; + +type PreparedRecordingStart = { + outPath: string; + resolvedOut: string; + recordingBase: RecordingBase; +}; +type RecordingQualityInput = Parameters[0]; +type RecordingStartBackend = ReturnType; +type RecordingStartPlan = PreparedRecordingStart & { + backend: RecordingStartBackend; + fpsFlag: number | undefined; +}; + function buildRecordTraceDeps(): RecordTraceDeps { return { runCmd: async (cmd, args, options) => @@ -76,30 +107,125 @@ function buildRecordingBase(req: DaemonRequest, outPath: string): RecordingBase // --- Start recording orchestrator --- -// fallow-ignore-next-line complexity -async function startRecording(params: { +async function startRecording(params: StartRecordingParams): Promise { + const { req, sessionName, sessionStore, activeSession, device, logPath, deps } = params; + const startPlan = resolveRecordingStartPlan(params); + if (!('backend' in startPlan)) return startPlan; + + const recording = await startPlan.backend.start({ + req, + activeSession, + sessionStore, + device, + logPath, + deps, + fpsFlag: startPlan.fpsFlag, + recordingBase: startPlan.recordingBase, + resolvedOut: startPlan.resolvedOut, + }); + + return persistStartedRecording({ + req, + sessionName, + sessionStore, + activeSession, + recording, + outPath: startPlan.outPath, + }); +} + +function resolveRecordingStartPlan( + params: StartRecordingParams, +): DaemonResponse | RecordingStartPlan { + const { req, activeSession, device } = params; + const backend = resolveRecordingBackendForDevice(device); + const startError = validateRecordingStartRequest({ req, activeSession, device, backend }); + if (startError) return startError; + + return { + ...prepareRecordingStart(req, backend), + backend, + fpsFlag: req.flags?.fps, + }; +} + +function validateRecordingStartRequest(params: { req: DaemonRequest; - sessionName: string; - sessionStore: SessionStore; activeSession: SessionState; device: SessionState['device']; - logPath?: string; - deps: RecordTraceDeps; -}): Promise { - const { req, sessionName, sessionStore, activeSession, device, logPath, deps } = params; - - if (activeSession.recording) { - return errorResponse('INVALID_ARGS', 'recording already in progress'); + backend: RecordingStartBackend; +}): DaemonResponse | null { + const { req, activeSession, device, backend } = params; + const validators = [ + () => validateNoActiveRecording(activeSession), + () => backend.validateStart?.(req) ?? null, + () => + validateRecordingStartFlags({ + fpsFlag: req.flags?.fps, + qualityFlag: req.flags?.quality, + maxSizeFlag: req.flags?.screenshotMaxSize, + }), + () => requireCommandSupported('record', device), + ]; + for (const validate of validators) { + const error = validate(); + if (error) return error; } + return null; +} - const fpsFlag = req.flags?.fps; - const qualityFlag = req.flags?.quality; - const maxSizeFlag = req.flags?.screenshotMaxSize; - const backend = resolveRecordingBackendForDevice(device); - const platformValidationError = backend.validateStart?.(req) ?? null; - if (platformValidationError) { - return platformValidationError; +function persistStartedRecording(params: { + req: DaemonRequest; + sessionName: string; + sessionStore: SessionStore; + activeSession: SessionState; + recording: Awaited>; + outPath: string; +}): DaemonResponse { + const { req, sessionName, sessionStore, activeSession, recording, outPath } = params; + if ('ok' in recording) { + return recording; } + + activeSession.recording = recording; + sessionStore.set(sessionName, activeSession); + const sessionStateDir = sessionStore.ensureSessionDir(sessionName); + recordSessionAction(sessionStore, activeSession, req, req.command, { + action: 'start', + showTouches: recording.showTouches, + }); + + return { + ok: true, + data: { + recording: 'started', + outPath: recording.clientOutPath ?? outPath, + sessionStateDir, + showTouches: recording.showTouches, + }, + }; +} + +function validateNoActiveRecording(activeSession: SessionState): DaemonResponse | null { + return activeSession.recording + ? errorResponse('INVALID_ARGS', 'recording already in progress') + : null; +} + +function validateRecordingStartFlags(flags: { + fpsFlag: number | undefined; + qualityFlag: RecordingQualityInput; + maxSizeFlag: number | undefined; +}): DaemonResponse | null { + const { fpsFlag, qualityFlag, maxSizeFlag } = flags; + return ( + validateRecordingFpsFlag(fpsFlag) ?? + validateRecordingQualityFlag(qualityFlag) ?? + validateRecordingMaxSizeFlag(maxSizeFlag) + ); +} + +function validateRecordingFpsFlag(fpsFlag: number | undefined): DaemonResponse | null { if ( fpsFlag !== undefined && (!Number.isInteger(fpsFlag) || @@ -111,6 +237,10 @@ async function startRecording(params: { `fps must be an integer between ${IOS_DEVICE_RECORD_MIN_FPS} and ${IOS_DEVICE_RECORD_MAX_FPS}`, ); } + return null; +} + +function validateRecordingQualityFlag(qualityFlag: RecordingQualityInput): DaemonResponse | null { if ( qualityFlag !== undefined && recordingQualityInputToExportQuality(qualityFlag) === undefined @@ -120,71 +250,33 @@ async function startRecording(params: { `quality must be one of: ${RECORDING_EXPORT_QUALITIES.join(', ')} (legacy numeric values 5-10 are accepted)`, ); } + return null; +} + +function validateRecordingMaxSizeFlag(maxSizeFlag: number | undefined): DaemonResponse | null { if (maxSizeFlag !== undefined && (!Number.isInteger(maxSizeFlag) || maxSizeFlag < 1)) { return errorResponse('INVALID_ARGS', 'max-size must be a positive integer'); } - const unsupported = requireCommandSupported('record', device); - if (unsupported) return unsupported; + return null; +} +function prepareRecordingStart( + req: DaemonRequest, + backend: ReturnType, +): PreparedRecordingStart { const outPath = backend.resolveOutputPath({ req }); const resolvedOut = SessionStore.expandHome(outPath, req.meta?.cwd); const recordingBase = buildRecordingBase(req, resolvedOut); fs.mkdirSync(path.dirname(resolvedOut), { recursive: true }); fs.rmSync(resolvedOut, { force: true }); - - const recording = await backend.start({ - req, - activeSession, - sessionStore, - device, - logPath, - deps, - fpsFlag, - recordingBase, - resolvedOut, - }); - - if ('ok' in recording) { - return recording; - } - - activeSession.recording = recording; - sessionStore.set(sessionName, activeSession); - const sessionStateDir = sessionStore.ensureSessionDir(sessionName); - recordSessionAction(sessionStore, activeSession, req, req.command, { - action: 'start', - showTouches: recording.showTouches, - }); - - return { - ok: true, - data: { - recording: 'started', - outPath: recording.clientOutPath ?? outPath, - sessionStateDir, - showTouches: recording.showTouches, - }, - }; + return { outPath, resolvedOut, recordingBase }; } -async function stopRecording(params: { - req: DaemonRequest; - sessionStore: SessionStore; - activeSession: SessionState; - device: SessionState['device']; - logPath?: string; - deps: RecordTraceDeps; -}): Promise { - const { req, sessionStore, activeSession, device, logPath, deps } = params; +async function stopRecording(params: StopRecordingParams): Promise { + const { req, activeSession, device, logPath, deps } = params; - let recording = activeSession.recording; - if (!recording) { - const recovered = await recoverMissingRecordingState(); - if (recovered && 'ok' in recovered) { - return recovered; - } - recording = recovered ?? undefined; - } + const recording = await resolveRecordingToStop(params); + if (recording && 'ok' in recording) return recording; if (!recording) { return errorResponse('INVALID_ARGS', 'no active recording'); } @@ -205,49 +297,81 @@ async function stopRecording(params: { return stopError; } - if (invalidatedReason && recording.platform === 'ios' && recording.showTouches) { - recording.overlayWarning ??= `overlay unavailable: ${invalidatedReason}`; - } else if (invalidatedReason) { - return errorResponse('COMMAND_FAILED', invalidatedReason); - } + const invalidatedError = applyRecordingInvalidation(recording, invalidatedReason); + if (invalidatedError) return invalidatedError; return buildRecordStopResponse(recording); +} - async function recoverMissingRecordingState(): Promise< - DaemonResponse | NonNullable | null - > { - if (hasActiveRecordingSession(sessionStore)) { - return null; - } +async function resolveRecordingToStop( + params: StopRecordingParams, +): Promise | null> { + if (params.activeSession.recording) { + return params.activeSession.recording; + } + return await recoverMissingRecordingState(params); +} - const backend = resolveRecordingBackendForDevice(device); - if (!backend.recoverMissingStop) { - return null; - } +async function recoverMissingRecordingState( + params: StopRecordingParams, +): Promise | null> { + const { req, sessionStore, activeSession, device, logPath, deps } = params; + if (hasActiveRecordingSession(sessionStore)) { + return null; + } - const outPath = backend.resolveOutputPath({ req }); - const resolvedOut = SessionStore.expandHome(outPath, req.meta?.cwd); - const recordingBase = buildRecordingBase(req, resolvedOut); + const backend = resolveRecordingBackendForDevice(device); + if (!backend.recoverMissingStop) { + return null; + } - const recovered = await backend.recoverMissingStop({ - req, - activeSession, - sessionStore, - device, - logPath, - deps, - recordingBase, - resolvedOut, - }); - if (!recovered) { - return null; - } - if (!('ok' in recovered)) { - fs.mkdirSync(path.dirname(resolvedOut), { recursive: true }); - fs.rmSync(resolvedOut, { force: true }); - } - return recovered; + const { resolvedOut, recordingBase } = prepareRecoveredRecording(req, backend); + const recovered = await backend.recoverMissingStop({ + req, + activeSession, + sessionStore, + device, + logPath, + deps, + recordingBase, + resolvedOut, + }); + if (!recovered) { + return null; + } + if (!('ok' in recovered)) { + resetRecoveredRecordingOutput(resolvedOut); + } + return recovered; +} + +function prepareRecoveredRecording( + req: DaemonRequest, + backend: ReturnType, +): Pick { + const outPath = backend.resolveOutputPath({ req }); + const resolvedOut = SessionStore.expandHome(outPath, req.meta?.cwd); + const recordingBase = buildRecordingBase(req, resolvedOut); + return { resolvedOut, recordingBase }; +} + +function resetRecoveredRecordingOutput(resolvedOut: string): void { + fs.mkdirSync(path.dirname(resolvedOut), { recursive: true }); + fs.rmSync(resolvedOut, { force: true }); +} + +function applyRecordingInvalidation( + recording: NonNullable, + invalidatedReason: string | undefined, +): DaemonResponse | null { + if (!invalidatedReason) { + return null; + } + if (recording.platform === 'ios' && recording.showTouches) { + recording.overlayWarning ??= `overlay unavailable: ${invalidatedReason}`; + return null; } + return errorResponse('COMMAND_FAILED', invalidatedReason); } function hasActiveRecordingSession(sessionStore: SessionStore): boolean { diff --git a/test/integration/provider-scenarios/android-recording.test.ts b/test/integration/provider-scenarios/android-recording.test.ts index 5cce9dc2c..7c07fde06 100644 --- a/test/integration/provider-scenarios/android-recording.test.ts +++ b/test/integration/provider-scenarios/android-recording.test.ts @@ -18,194 +18,271 @@ import { withProviderScenarioTempDir, } from './harness.ts'; +type ProviderScenarioDaemon = Awaited>; +type PullCall = { remotePath: string; localPath: string }; + test('Provider-backed integration Android recording flow uses scripted ADB provider pull capability', async () => { await withProviderScenarioTempDir( 'agent-device-provider-scenario-android-record-', - async (tmpDir) => { - const recordingPath = path.join(tmpDir, 'recording.mp4'); - const adbCalls: string[][] = []; - const pullCalls: Array<{ remotePath: string; localPath: string }> = []; - const adbProvider: AndroidAdbProvider = { - exec: async (args) => { - adbCalls.push([...args]); - return androidAdbResult(args); - }, - pull: async (remotePath, localPath) => { - pullCalls.push({ remotePath, localPath }); - fs.writeFileSync(localPath, likelyPlayableMp4Container()); - return { stdout: '', stderr: '', exitCode: 0 }; - }, - }; - const daemon = await createProviderScenarioHarness({ - androidAdbProvider: () => adbProvider, - deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], - }); + runAndroidRecordingFlowScenario, + ); +}); + +test('Provider-backed integration Android record stop recovers missing daemon recording state', async () => { + await withProviderScenarioTempDir( + 'agent-device-provider-scenario-android-record-recovery-', + runAndroidRecordingRecoveryScenario, + ); +}); + +test('Provider-backed integration Android record start without a session scopes default-device providers', async () => { + await withMockedAdb( + 'agent-device-provider-scenario-android-sessionless-record-', + runAndroidSessionlessRecordingWithMockedAdb, + ); +}); + +async function runAndroidRecordingFlowScenario(tmpDir: string): Promise { + const context = await createAndroidRecordingFlowContext(tmpDir); + await withAndroidProviderScenarioEnv(tmpDir, async () => { + try { + await exerciseAndroidRecordingFlow(context); + assertAndroidRecordingFlow(context); + } finally { + await context.daemon.close(); + } + }); +} - const previousPath = process.env.PATH; - const previousSwiftCacheDir = process.env.AGENT_DEVICE_SWIFT_CACHE_DIR; - process.env.PATH = tmpDir; - process.env.AGENT_DEVICE_SWIFT_CACHE_DIR = path.join(tmpDir, 'swift-cache'); +async function createAndroidRecordingFlowContext(tmpDir: string): Promise<{ + recordingPath: string; + adbCalls: string[][]; + pullCalls: PullCall[]; + daemon: ProviderScenarioDaemon; +}> { + const recordingPath = path.join(tmpDir, 'recording.mp4'); + const adbCalls: string[][] = []; + const pullCalls: PullCall[] = []; + const daemon = await createProviderScenarioHarness({ + androidAdbProvider: () => createPullingAndroidProvider({ adbCalls, pullCalls }), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); + return { recordingPath, adbCalls, pullCalls, daemon }; +} + +async function exerciseAndroidRecordingFlow(context: { + recordingPath: string; + daemon: ProviderScenarioDaemon; +}): Promise { + const open = await context.daemon.callCommand('open', ['settings'], { + platform: 'android', + serial: PROVIDER_SCENARIO_ANDROID.id, + }); + assertRpcOk(open); - try { - const open = await daemon.callCommand('open', ['settings'], { - platform: 'android', - serial: PROVIDER_SCENARIO_ANDROID.id, - }); - assertRpcOk(open); + const recordStart = await context.daemon.callCommand('record', ['start', context.recordingPath], { + hideTouches: true, + screenshotMaxSize: 1344, + quality: 'high', + }); + assertRecordingStarted(recordStart, { showTouches: false }); - const recordStart = await daemon.callCommand('record', ['start', recordingPath], { - hideTouches: true, - screenshotMaxSize: 1344, - quality: 'high', - }); - assertRecordingStarted(recordStart, { showTouches: false }); + const recordStop = await context.daemon.callCommand('record', ['stop']); + assertRecordingStopped(recordStop, context.recordingPath, { showTouches: false }); +} - const recordStop = await daemon.callCommand('record', ['stop']); - assertRecordingStopped(recordStop, recordingPath, { showTouches: false }); - assert.equal(fs.existsSync(recordingPath), true); +function assertAndroidRecordingFlow(context: { + recordingPath: string; + adbCalls: string[][]; + pullCalls: PullCall[]; +}): void { + const { recordingPath, adbCalls, pullCalls } = context; + assert.equal(fs.existsSync(recordingPath), true); + assertCommandCall(adbCalls, ['shell', 'wm', 'size']); + assert.ok(adbCalls.some((args) => isAndroidHighQualityScreenrecordStartCommand(args.join(' ')))); + assertCommandCall(adbCalls, ['shell', 'kill', '-2', '4321']); + assert.equal(pullCalls.length, 1); + assert.match(pullCalls[0]?.remotePath ?? '', /^\/sdcard\/agent-device-recording-\d+\.mp4$/); + assert.equal(pullCalls[0]?.localPath, recordingPath); + assert.ok(adbCalls.some((args) => args[0] === 'shell' && args[1] === 'rm')); + assert.equal( + adbCalls.some((args) => args[0] === 'pull'), + false, + ); +} + +async function runAndroidRecordingRecoveryScenario(tmpDir: string): Promise { + const context = await createAndroidRecordingRecoveryContext(); + await withAndroidProviderScenarioEnv(tmpDir, async () => { + try { + const outPath = await exerciseAndroidRecordingRecoveryStop(context, tmpDir); + assertAndroidRecordingRecovery(context, outPath); + } finally { + await context.daemon.close(); + } + }); +} + +async function createAndroidRecordingRecoveryContext(): Promise<{ + remotePath: string; + adbCalls: string[][]; + pullCalls: PullCall[]; + daemon: ProviderScenarioDaemon; +}> { + const remotePath = '/sdcard/agent-device-recording-123456789.mp4'; + const adbCalls: string[][] = []; + const pullCalls: PullCall[] = []; + const daemon = await createProviderScenarioHarness({ + androidAdbProvider: () => + createPullingAndroidProvider({ + adbCalls, + pullCalls, + exec: (args) => androidRecoveryAdbResult(args, remotePath), + }), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); + return { remotePath, adbCalls, pullCalls, daemon }; +} - assertCommandCall(adbCalls, ['shell', 'wm', 'size']); - assert.ok( - adbCalls.some((args) => - /^shell screenrecord --size 756x1344 --bit-rate 20000000 \/sdcard\/agent-device-recording-\d+\.mp4 >\/dev\/null 2>&1 & echo \$!$/.test( - args.join(' '), - ), - ), - JSON.stringify(adbCalls), - ); - assertCommandCall(adbCalls, ['shell', 'kill', '-2', '4321']); - assert.equal(pullCalls.length, 1); - assert.match(pullCalls[0]?.remotePath ?? '', /^\/sdcard\/agent-device-recording-\d+\.mp4$/); - assert.equal(pullCalls[0]?.localPath, recordingPath); - assert.ok(adbCalls.some((args) => args[0] === 'shell' && args[1] === 'rm')); - assert.equal( - adbCalls.some((args) => args[0] === 'pull'), - false, - ); - } finally { - await daemon.close(); - restoreEnv('PATH', previousPath); - restoreEnv('AGENT_DEVICE_SWIFT_CACHE_DIR', previousSwiftCacheDir); - } +async function exerciseAndroidRecordingRecoveryStop( + context: { daemon: ProviderScenarioDaemon }, + tmpDir: string, +): Promise { + const recordStop = await context.daemon.callCommand( + 'record', + ['stop'], + { + platform: 'android', + serial: PROVIDER_SCENARIO_ANDROID.id, }, + { meta: { cwd: tmpDir } }, ); -}); + const data = assertRpcOk<{ recording?: unknown; outPath?: unknown; warning?: unknown }>( + recordStop, + ); + assert.equal(data.recording, 'stopped'); + assert.match(String(data.warning), /Recovered Android recording/); + return requireStringOutPath(data.outPath); +} -test('Provider-backed integration Android record stop recovers missing daemon recording state', async () => { +function assertAndroidRecordingRecovery( + context: { remotePath: string; adbCalls: string[][]; pullCalls: PullCall[] }, + outPath: string, +): void { + const { remotePath, adbCalls, pullCalls } = context; + assert.match(outPath, /\/recording-\d+\.mp4$/); + assert.equal(fs.existsSync(outPath), true); + assertCommandCall(adbCalls, ['shell', 'ps', '-A', '-o', 'pid=,args=']); + assertCommandCall(adbCalls, ['shell', 'kill', '-2', '4321']); + assert.equal(pullCalls.length, 1); + assert.deepEqual(pullCalls[0], { remotePath, localPath: outPath }); + assertCommandCall(adbCalls, ['shell', 'rm', '-f', remotePath]); +} + +async function runAndroidSessionlessRecordingWithMockedAdb(logPath: string): Promise { await withProviderScenarioTempDir( - 'agent-device-provider-scenario-android-record-recovery-', - async (tmpDir) => { - const adbCalls: string[][] = []; - const pullCalls: Array<{ remotePath: string; localPath: string }> = []; - const remotePath = '/sdcard/agent-device-recording-123456789.mp4'; - const adbProvider: AndroidAdbProvider = { - exec: async (args) => { - adbCalls.push([...args]); - if (args.join(' ') === 'shell ps -A -o pid=,args=') { - return { - stdout: `4321 screenrecord --bit-rate 8000000 ${remotePath}\n`, - stderr: '', - exitCode: 0, - }; - } - return androidAdbResult(args); - }, - pull: async (from, to) => { - pullCalls.push({ remotePath: from, localPath: to }); - fs.writeFileSync(to, likelyPlayableMp4Container()); - return { stdout: '', stderr: '', exitCode: 0 }; - }, - }; - const daemon = await createProviderScenarioHarness({ - androidAdbProvider: () => adbProvider, - deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], - }); + 'agent-device-provider-scenario-android-sessionless-record-', + async (tmpDir) => await runAndroidSessionlessRecordingScenario(tmpDir, logPath), + ); +} - const previousPath = process.env.PATH; - const previousSwiftCacheDir = process.env.AGENT_DEVICE_SWIFT_CACHE_DIR; - process.env.PATH = tmpDir; - process.env.AGENT_DEVICE_SWIFT_CACHE_DIR = path.join(tmpDir, 'swift-cache'); +async function runAndroidSessionlessRecordingScenario( + tmpDir: string, + logPath: string, +): Promise { + const recordingPath = path.join(tmpDir, 'sessionless-recording.mp4'); + const adbCalls: string[][] = []; + const daemon = await createProviderScenarioHarness({ + androidAdbProvider: () => createRecordingOnlyAndroidProvider(adbCalls), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); - try { - const recordStop = await daemon.callCommand( - 'record', - ['stop'], - { - platform: 'android', - serial: PROVIDER_SCENARIO_ANDROID.id, - }, - { meta: { cwd: tmpDir } }, - ); - const data = assertRpcOk<{ - recording?: unknown; - outPath?: unknown; - warning?: unknown; - }>(recordStop); - assert.equal(data.recording, 'stopped'); - const outPath = data.outPath; - assert.equal(typeof outPath, 'string'); - if (typeof outPath !== 'string') { - throw new Error(`expected string outPath, got ${String(outPath)}`); - } - assert.match(outPath, /\/recording-\d+\.mp4$/); - assert.match(String(data.warning), /Recovered Android recording/); - assert.equal(fs.existsSync(outPath), true); + try { + const recordStart = await daemon.callCommand('record', ['start', recordingPath]); + assertRecordingStarted(recordStart, { showTouches: true }); + assertAndroidSessionlessRecording(adbCalls, logPath); + } finally { + await daemon.close(); + } +} - assertCommandCall(adbCalls, ['shell', 'ps', '-A', '-o', 'pid=,args=']); - assertCommandCall(adbCalls, ['shell', 'kill', '-2', '4321']); - assert.equal(pullCalls.length, 1); - assert.deepEqual(pullCalls[0], { remotePath, localPath: outPath }); - assertCommandCall(adbCalls, ['shell', 'rm', '-f', remotePath]); - } finally { - await daemon.close(); - restoreEnv('PATH', previousPath); - restoreEnv('AGENT_DEVICE_SWIFT_CACHE_DIR', previousSwiftCacheDir); - } - }, +function assertAndroidSessionlessRecording(adbCalls: string[][], logPath: string): void { + assert.ok( + adbCalls.some((args) => isAndroidDefaultScreenrecordStartCommand(args.join(' '))), + JSON.stringify(adbCalls), ); -}); + assert.equal( + adbCalls.some((args) => args.join(' ') === 'shell wm size'), + false, + ); + assert.deepEqual(readLoggedArgs(logPath), []); +} -test('Provider-backed integration Android record start without a session scopes default-device providers', async () => { - await withMockedAdb( - 'agent-device-provider-scenario-android-sessionless-record-', - async (logPath) => { - await withProviderScenarioTempDir( - 'agent-device-provider-scenario-android-sessionless-record-', - async (tmpDir) => { - const recordingPath = path.join(tmpDir, 'sessionless-recording.mp4'); - const adbCalls: string[][] = []; - const adbProvider: AndroidAdbProvider = { - exec: async (args) => { - adbCalls.push([...args]); - return androidAdbResult(args); - }, - }; - const daemon = await createProviderScenarioHarness({ - androidAdbProvider: () => adbProvider, - deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], - }); +async function withAndroidProviderScenarioEnv( + tmpDir: string, + runScenario: () => Promise, +): Promise { + const previousPath = process.env.PATH; + const previousSwiftCacheDir = process.env.AGENT_DEVICE_SWIFT_CACHE_DIR; + process.env.PATH = tmpDir; + process.env.AGENT_DEVICE_SWIFT_CACHE_DIR = path.join(tmpDir, 'swift-cache'); + try { + await runScenario(); + } finally { + restoreEnv('PATH', previousPath); + restoreEnv('AGENT_DEVICE_SWIFT_CACHE_DIR', previousSwiftCacheDir); + } +} - try { - const recordStart = await daemon.callCommand('record', ['start', recordingPath]); +function createPullingAndroidProvider(params: { + adbCalls: string[][]; + pullCalls: PullCall[]; + exec?: (args: string[]) => ReturnType; +}): AndroidAdbProvider { + const { adbCalls, pullCalls, exec = androidAdbResult } = params; + return { + exec: async (args) => { + adbCalls.push([...args]); + return exec(args); + }, + pull: async (remotePath, localPath) => { + pullCalls.push({ remotePath, localPath }); + fs.writeFileSync(localPath, likelyPlayableMp4Container()); + return { stdout: '', stderr: '', exitCode: 0 }; + }, + }; +} - assertRecordingStarted(recordStart, { showTouches: true }); - assert.ok( - adbCalls.some((args) => isAndroidDefaultScreenrecordStartCommand(args.join(' '))), - JSON.stringify(adbCalls), - ); - assert.equal( - adbCalls.some((args) => args.join(' ') === 'shell wm size'), - false, - ); - assert.deepEqual(readLoggedArgs(logPath), []); - } finally { - await daemon.close(); - } - }, - ); +function createRecordingOnlyAndroidProvider(adbCalls: string[][]): AndroidAdbProvider { + return { + exec: async (args) => { + adbCalls.push([...args]); + return androidAdbResult(args); }, - ); -}); + }; +} + +function androidRecoveryAdbResult( + args: string[], + remotePath: string, +): ReturnType { + if (args.join(' ') === 'shell ps -A -o pid=,args=') { + return { + stdout: `4321 screenrecord --bit-rate 8000000 ${remotePath}\n`, + stderr: '', + exitCode: 0, + }; + } + return androidAdbResult(args); +} + +function requireStringOutPath(value: unknown): string { + assert.equal(typeof value, 'string'); + if (typeof value !== 'string') { + throw new Error(`expected string outPath, got ${String(value)}`); + } + return value; +} function androidAdbResult(args: string[]): { stdout: string; @@ -249,6 +326,12 @@ function isAndroidScreenrecordStartCommand(command: string): boolean { ); } +function isAndroidHighQualityScreenrecordStartCommand(command: string): boolean { + return /^shell screenrecord --size 756x1344 --bit-rate 20000000 \/sdcard\/agent-device-recording-\d+\.mp4 >\/dev\/null 2>&1 & echo \$!$/.test( + command, + ); +} + function isAndroidDefaultScreenrecordStartCommand(command: string): boolean { return /^shell screenrecord --bit-rate 8000000 \/sdcard\/agent-device-recording-\d+\.mp4 >\/dev\/null 2>&1 & echo \$!$/.test( command, From bc2bd44c0680e8bcea05aac35a492d71f3978c07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Mon, 6 Jul 2026 13:58:47 +0200 Subject: [PATCH 3/3] fix: address android recording recovery feedback --- .../handlers/record-trace-android-recovery.ts | 38 +++- src/daemon/handlers/record-trace-android.ts | 12 +- src/daemon/handlers/record-trace-recording.ts | 6 +- .../android-recording.test.ts | 164 ++++++++++++++++++ .../integration/provider-scenarios/harness.ts | 14 +- 5 files changed, 222 insertions(+), 12 deletions(-) diff --git a/src/daemon/handlers/record-trace-android-recovery.ts b/src/daemon/handlers/record-trace-android-recovery.ts index 6e95f1ccc..101181423 100644 --- a/src/daemon/handlers/record-trace-android-recovery.ts +++ b/src/daemon/handlers/record-trace-android-recovery.ts @@ -133,11 +133,20 @@ async function readAndroidRecoveryMetadata( } const metadata = parseAndroidRecoveryMetadata(result.stdout); if (!metadata) { + await cleanupAndroidRecoveryMetadataPath({ + deviceId, + metadataPath, + phase: 'record_stop_android_recovery_metadata_invalid_cleanup_failed', + }); continue; } - if (await isLiveRecoverableAndroidScreenrecord(deviceId, metadata)) { + const liveness = await checkLiveRecoverableAndroidScreenrecord(deviceId, metadata); + if (liveness === 'live') { return metadata; } + if (liveness === 'uncertain') { + continue; + } await cleanupAndroidRecoveryMetadataPath({ deviceId, metadataPath, @@ -147,10 +156,10 @@ async function readAndroidRecoveryMetadata( return undefined; } -async function isLiveRecoverableAndroidScreenrecord( +async function checkLiveRecoverableAndroidScreenrecord( deviceId: string, metadata: AndroidRecordingRecoveryMetadata, -): Promise { +): Promise<'live' | 'stale' | 'uncertain'> { const result = await runAndroidRecoveryAdb( deviceId, ['shell', 'ps', '-o', 'pid=,args=', '-p', metadata.remotePid], @@ -160,15 +169,34 @@ async function isLiveRecoverableAndroidScreenrecord( }, ); if (result.exitCode !== 0) { - return false; + emitDiagnostic({ + level: 'debug', + phase: 'record_stop_android_recovery_metadata_probe_uncertain', + data: { + deviceId, + remotePid: metadata.remotePid, + remotePath: metadata.remotePath, + exitCode: result.exitCode, + stdout: result.stdout.trim(), + stderr: result.stderr.trim(), + }, + }); + return 'uncertain'; } - return result.stdout + const sawPid = result.stdout + .split(/\r?\n/) + .some((line) => line.trim().startsWith(metadata.remotePid)); + const matched = result.stdout .split(/\r?\n/) .map(parseRecoverableAndroidScreenrecord) .some( (candidate) => candidate?.remotePid === metadata.remotePid && candidate.remotePath === metadata.remotePath, ); + if (matched) { + return 'live'; + } + return sawPid ? 'uncertain' : 'stale'; } async function findRecoverableAndroidScreenrecord( diff --git a/src/daemon/handlers/record-trace-android.ts b/src/daemon/handlers/record-trace-android.ts index 01563b4c1..0f998eb2d 100644 --- a/src/daemon/handlers/record-trace-android.ts +++ b/src/daemon/handlers/record-trace-android.ts @@ -422,7 +422,7 @@ async function finishCurrentAndroidRecordingChunk(params: { const { device, recording, waitForRemoteFileStability = true } = params; const wasRunningBeforeStop = await isAndroidProcessRunning(device.id, recording.remotePid); if (!wasRunningBeforeStop) { - recording.warning ??= resolveAndroidScreenrecordLimitWarning(recording); + appendAndroidRecordingWarning(recording, resolveAndroidScreenrecordLimitWarning(recording)); } const stopResult = await runAndroidRecordingAdb( @@ -615,3 +615,13 @@ function formatAndroidStopFailure( ): string { return buildRecordStopFailure(error, recording, stopRequestedAt).message; } + +function appendAndroidRecordingWarning( + recording: AndroidRecording, + warning: string | undefined, +): void { + if (!warning || recording.warning?.includes(warning)) { + return; + } + recording.warning = recording.warning ? `${recording.warning} ${warning}` : warning; +} diff --git a/src/daemon/handlers/record-trace-recording.ts b/src/daemon/handlers/record-trace-recording.ts index 74038716a..a88ed9f68 100644 --- a/src/daemon/handlers/record-trace-recording.ts +++ b/src/daemon/handlers/record-trace-recording.ts @@ -316,7 +316,7 @@ async function recoverMissingRecordingState( params: StopRecordingParams, ): Promise | null> { const { req, sessionStore, activeSession, device, logPath, deps } = params; - if (hasActiveRecordingSession(sessionStore)) { + if (hasActiveRecordingSessionForDevice(sessionStore, device.id)) { return null; } @@ -374,9 +374,9 @@ function applyRecordingInvalidation( return errorResponse('COMMAND_FAILED', invalidatedReason); } -function hasActiveRecordingSession(sessionStore: SessionStore): boolean { +function hasActiveRecordingSessionForDevice(sessionStore: SessionStore, deviceId: string): boolean { for (const session of sessionStore.values()) { - if (session.recording) { + if (session.recording && session.device.id === deviceId) { return true; } } diff --git a/test/integration/provider-scenarios/android-recording.test.ts b/test/integration/provider-scenarios/android-recording.test.ts index 7c07fde06..7d2e897cb 100644 --- a/test/integration/provider-scenarios/android-recording.test.ts +++ b/test/integration/provider-scenarios/android-recording.test.ts @@ -8,6 +8,7 @@ import { assertCommandCall, assertRecordingStarted, assertRecordingStopped, + assertRpcError, assertRpcOk, } from './assertions.ts'; import { PROVIDER_SCENARIO_ANDROID } from './fixtures.ts'; @@ -35,6 +36,27 @@ test('Provider-backed integration Android record stop recovers missing daemon re ); }); +test('Provider-backed integration Android record stop recovers while another device is recording', async () => { + await withProviderScenarioTempDir( + 'agent-device-provider-scenario-android-record-cross-device-recovery-', + runAndroidCrossDeviceRecordingRecoveryScenario, + ); +}); + +test('Provider-backed integration Android record stop keeps valid metadata on uncertain liveness probe', async () => { + await withProviderScenarioTempDir( + 'agent-device-provider-scenario-android-record-uncertain-metadata-', + runAndroidUncertainMetadataScenario, + ); +}); + +test('Provider-backed integration Android record stop cleans corrupt recovery metadata', async () => { + await withProviderScenarioTempDir( + 'agent-device-provider-scenario-android-record-corrupt-metadata-', + runAndroidCorruptMetadataScenario, + ); +}); + test('Provider-backed integration Android record start without a session scopes default-device providers', async () => { await withMockedAdb( 'agent-device-provider-scenario-android-sessionless-record-', @@ -111,6 +133,57 @@ function assertAndroidRecordingFlow(context: { ); } +async function runAndroidCrossDeviceRecordingRecoveryScenario(tmpDir: string): Promise { + const otherAndroid = { ...PROVIDER_SCENARIO_ANDROID, id: 'emulator-5556', name: 'Pixel 8 B' }; + const remotePath = '/sdcard/agent-device-recording-123456789.mp4'; + const adbCalls: string[][] = []; + const pullCalls: PullCall[] = []; + const daemon = await createProviderScenarioHarness({ + androidAdbProvider: () => + createPullingAndroidProvider({ + adbCalls, + pullCalls, + exec: (args) => androidRecoveryAdbResult(args, remotePath), + }), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID, otherAndroid], + }); + + await withAndroidProviderScenarioEnv(tmpDir, async () => { + try { + const busyRecordingPath = path.join(tmpDir, 'busy-recording.mp4'); + const openBusy = await daemon.callCommand( + 'open', + ['settings'], + { platform: 'android', serial: otherAndroid.id }, + { session: 'busy' }, + ); + assertRpcOk(openBusy); + const startBusy = await daemon.callCommand( + 'record', + ['start', busyRecordingPath], + {}, + { session: 'busy' }, + ); + assertRecordingStarted(startBusy); + + const recordStop = await daemon.callCommand( + 'record', + ['stop'], + { platform: 'android', serial: PROVIDER_SCENARIO_ANDROID.id }, + { meta: { cwd: tmpDir } }, + ); + const data = assertRpcOk<{ recording?: unknown; outPath?: unknown }>(recordStop); + assert.equal(data.recording, 'stopped'); + assert.match(String(data.outPath), /\/recording-\d+\.mp4$/); + assert.equal(daemon.session('busy')?.recording !== undefined, true); + assertCommandCall(adbCalls, ['shell', 'ps', '-A', '-o', 'pid=,args=']); + assert.equal(pullCalls.length, 1); + } finally { + await daemon.close(); + } + }); +} + async function runAndroidRecordingRecoveryScenario(tmpDir: string): Promise { const context = await createAndroidRecordingRecoveryContext(); await withAndroidProviderScenarioEnv(tmpDir, async () => { @@ -162,6 +235,7 @@ async function exerciseAndroidRecordingRecoveryStop( ); assert.equal(data.recording, 'stopped'); assert.match(String(data.warning), /Recovered Android recording/); + assert.match(String(data.warning), /MP4 may be truncated/); return requireStringOutPath(data.outPath); } @@ -179,6 +253,96 @@ function assertAndroidRecordingRecovery( assertCommandCall(adbCalls, ['shell', 'rm', '-f', remotePath]); } +async function runAndroidUncertainMetadataScenario(_tmpDir: string): Promise { + const adbCalls: string[][] = []; + const remotePath = '/sdcard/agent-device-recording-123456789.mp4'; + 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({ + remotePath, + remotePid: '4321', + startedAt: 123456789, + }), + stderr: '', + exitCode: 0, + }; + } + if (command === 'shell cat /data/local/tmp/agent-device-recording-active.json') { + return { stdout: '', stderr: '', exitCode: 1 }; + } + if (command === 'shell ps -o pid=,args= -p 4321') { + return { stdout: '', stderr: 'transient ps failure', exitCode: 1 }; + } + if (command === 'shell ps -A -o pid=,args=') { + return { stdout: '', stderr: '', exitCode: 0 }; + } + return androidAdbResult(args); + }, + }), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); + + try { + const recordStop = await daemon.callCommand('record', ['stop'], { + platform: 'android', + serial: PROVIDER_SCENARIO_ANDROID.id, + }); + assertRpcError(recordStop, 'INVALID_ARGS', /no active recording/); + assert.equal( + adbCalls.some( + (args) => args.join(' ') === 'shell rm -f /sdcard/agent-device-recording-active.json', + ), + false, + ); + } finally { + await daemon.close(); + } +} + +async function runAndroidCorruptMetadataScenario(_tmpDir: string): Promise { + const adbCalls: string[][] = []; + 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: '{', stderr: '', exitCode: 0 }; + } + if (command === 'shell cat /data/local/tmp/agent-device-recording-active.json') { + return { stdout: '', stderr: '', exitCode: 1 }; + } + if (command === 'shell ps -A -o pid=,args=') { + return { stdout: '', stderr: '', exitCode: 0 }; + } + return androidAdbResult(args); + }, + }), + deviceInventoryProvider: async () => [PROVIDER_SCENARIO_ANDROID], + }); + + try { + const recordStop = await daemon.callCommand('record', ['stop'], { + platform: 'android', + serial: PROVIDER_SCENARIO_ANDROID.id, + }); + assertRpcError(recordStop, 'INVALID_ARGS', /no active recording/); + assertCommandCall(adbCalls, [ + 'shell', + 'rm', + '-f', + '/sdcard/agent-device-recording-active.json', + ]); + } finally { + await daemon.close(); + } +} + async function runAndroidSessionlessRecordingWithMockedAdb(logPath: string): Promise { await withProviderScenarioTempDir( 'agent-device-provider-scenario-android-sessionless-record-', diff --git a/test/integration/provider-scenarios/harness.ts b/test/integration/provider-scenarios/harness.ts index 8ba90a4a5..f5abb052a 100644 --- a/test/integration/provider-scenarios/harness.ts +++ b/test/integration/provider-scenarios/harness.ts @@ -24,7 +24,11 @@ export type ProviderScenarioHarness = { command: string, positionals?: string[], flags?: DaemonRequest['flags'], - options?: { meta?: DaemonRequest['meta']; runtime?: DaemonRequest['runtime'] }, + options?: { + meta?: DaemonRequest['meta']; + runtime?: DaemonRequest['runtime']; + session?: string; + }, ) => Promise; client: () => AgentDeviceClient; session: (name?: string) => SessionState | undefined; @@ -129,11 +133,15 @@ function commandRequest( command: string, positionals: string[] = [], flags: DaemonRequest['flags'] = {}, - options: { meta?: DaemonRequest['meta']; runtime?: DaemonRequest['runtime'] } = {}, + options: { + meta?: DaemonRequest['meta']; + runtime?: DaemonRequest['runtime']; + session?: string; + } = {}, ): DaemonRequest { return { token: PROVIDER_SCENARIO_TOKEN, - session: 'default', + session: options.session ?? 'default', command, positionals, flags,