diff --git a/packages/contracts/src/facades/recording.ts b/packages/contracts/src/facades/recording.ts index 0175a29c10..7674d119aa 100644 --- a/packages/contracts/src/facades/recording.ts +++ b/packages/contracts/src/facades/recording.ts @@ -5,7 +5,11 @@ export { recordingQualityInputToExportQuality, } from '../recording-export-quality.ts'; export type { RecordingExportQuality } from '../recording-export-quality.ts'; -export { RECORDING_SCOPE_VALUES, isWholeScreenRecordingScope } from '../recording-scope.ts'; +export { + RECORDING_SCOPE_VALUES, + isRecordingScope, + isWholeScreenRecordingScope, +} from '../recording-scope.ts'; export type { RecordingScope } from '../recording-scope.ts'; export type { RecordingAppIdentity, diff --git a/packages/contracts/src/recording-scope.ts b/packages/contracts/src/recording-scope.ts index e53ec97293..922e87c4de 100644 --- a/packages/contracts/src/recording-scope.ts +++ b/packages/contracts/src/recording-scope.ts @@ -2,6 +2,10 @@ export const RECORDING_SCOPE_VALUES = ['app', 'device', 'system'] as const; export type RecordingScope = (typeof RECORDING_SCOPE_VALUES)[number]; +export function isRecordingScope(value: unknown): value is RecordingScope { + return RECORDING_SCOPE_VALUES.some((scope) => scope === value); +} + export function isWholeScreenRecordingScope(scope: RecordingScope): boolean { return scope === 'device' || scope === 'system'; } diff --git a/src/cli-schema/cli-help.ts b/src/cli-schema/cli-help.ts index 0ac8b4d06e..f7b8dbc1cf 100644 --- a/src/cli-schema/cli-help.ts +++ b/src/cli-schema/cli-help.ts @@ -237,7 +237,7 @@ Batch: agent-device test ./e2e/maestro --maestro --device udid1,emulator-5554 --shard-all 2 Recording: - record start/stop. Default scope is app (needs an active open session); use --scope device/system for whole-screen capture spanning multiple apps/home/settings. --quality medium|high on Android and Apple targets. stop burns touch overlays into the video by default; --hide-touches skips that for the fastest raw recording, and is recommended for gesture-heavy iOS simulator proof videos since overlay timing depends on a stable runner session. Android adb screenrecord has a 180s limit, so long Android recordings return as multiple MP4 chunks while the daemon stays alive; after a daemon restart, record stop recovers only manifest-owned chunks. + record start/stop. Default scope is app (needs an active open session); use --scope device/system for whole-screen capture spanning multiple apps/home/settings. --quality medium|high on Android and Apple targets. stop burns touch overlays into the video by default; --hide-touches skips that for the fastest raw recording, and is recommended for gesture-heavy iOS simulator proof videos since overlay timing depends on a stable runner session. Android adb screenrecord has a 180s limit, so long Android recordings return as multiple MP4 chunks while the daemon stays alive; after a daemon restart, record stop recovers only manifest-owned chunks. record stop is safe to repeat: if its request window ended while the daemon was still exporting, running it again in that session returns the completed recording instead of starting a second one. Tracing: trace start ./trace.log, trace stop ./trace.log (path is positional, not --path).`, }, gestures: { diff --git a/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts b/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts index 43a3131975..0460f6cad6 100644 --- a/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts +++ b/src/daemon-client/__tests__/daemon-client-timeout-route.test.ts @@ -1,7 +1,7 @@ // Production-seam coverage for the real request-timeout route. // -// src/daemon-client/__tests__/daemon-client.test.ts covers `resolveRequestTimeoutHint` -// as a pure formatter, but a pure-formatter test cannot catch a bug in +// src/daemon-client/__tests__/daemon-client-timeout.test.ts covers +// `resolveRequestTimeoutHint` as a pure formatter, but a pure-formatter test cannot catch a bug in // CLEANUP ELIGIBILITY: whether `cleanupTimedOutIosRunnerBuilds` (the Apple // xcodebuild pkill sweep) actually runs. This file spies on the real // process-execution seam (`runCmdSync`, @agent-device/host-kit/command) and drives an diff --git a/src/daemon-client/__tests__/daemon-client-timeout.test.ts b/src/daemon-client/__tests__/daemon-client-timeout.test.ts new file mode 100644 index 0000000000..1c5658a2d9 --- /dev/null +++ b/src/daemon-client/__tests__/daemon-client-timeout.test.ts @@ -0,0 +1,146 @@ +// The pure hint formatter in src/daemon-client/daemon-client-timeout.ts: what a timed-out request +// tells the caller to do next. daemon-client-timeout-route.test.ts covers the same route at its +// production seam, where cleanup eligibility is decided; these assertions only fix the wording. + +import assert from 'node:assert/strict'; +import { test } from 'vitest'; +import { resolveRequestTimeoutHint } from '../daemon-client-timeout.ts'; + +test('request timeout hint only names Apple runner cleanup on actual evidence', () => { + // Before this change, handleRequestTimeout emitted Apple-specific hint + // wording for EVERY local timeout, regardless of the request's + // --platform. That was misleading for Android/web/Harmony sessions, which + // never had any Apple runner work to abort. + // + // The fix is evidence-based, not platform-guess-based: + // `appleCleanupEvidence` is true only when the request declared an + // AFFIRMATIVELY Apple platform (apple/ios/macos) or the pkill cleanup + // itself terminated a matching process — never from an undeclared or + // declared-non-Apple platform alone. (Why not trust the declared platform + // directly: it is not authoritative for session-bound execution — see + // `handleRequestTimeout`'s comment and the production-seam coverage in + // daemon-client-timeout-route.test.ts for the cleanup-eligibility half of + // this contract that this pure formatter test cannot prove.) + + // appleCleanupEvidence: true keeps the exact historical wording — nothing + // regresses for the true-Apple case. + assert.equal( + resolveRequestTimeoutHint({ + remote: false, + resetDaemon: false, + command: 'press', + appleCleanupEvidence: true, + }), + 'Retry with --debug and check daemon diagnostics logs. The timed-out press request was canceled and Apple runner work was aborted when detected; the daemon was kept alive so the session can still be closed or inspected.', + ); + assert.equal( + resolveRequestTimeoutHint({ + remote: false, + resetDaemon: true, + command: 'open', + appleCleanupEvidence: true, + }), + 'Retry with --debug and check daemon diagnostics logs. Timed-out Apple runner xcodebuild processes were terminated when detected.', + ); + assert.equal( + resolveRequestTimeoutHint({ + remote: false, + resetDaemon: false, + command: 'snapshot', + appleCleanupEvidence: true, + }), + 'Retry with --debug and check daemon diagnostics logs. The timed-out snapshot request was canceled and Apple runner work was aborted when detected; the daemon was kept alive so the session can still be closed or inspected. If this was the first Apple-platform snapshot on the device, run agent-device prepare ios-runner with the same --platform before snapshot/test so runner startup is handled explicitly.', + ); + + // appleCleanupEvidence: false — no Apple-runner claim in any branch, and + // the Apple-only iOS-prepare follow-up drops entirely. This is the + // motivating fix: it fires equally whether the platform was declared + // non-Apple OR left undeclared (the common session-bound case), because + // neither is Apple evidence on its own. + assert.equal( + resolveRequestTimeoutHint({ + remote: false, + resetDaemon: false, + command: 'press', + appleCleanupEvidence: false, + }), + 'Retry with --debug and check daemon diagnostics logs. The timed-out press request was canceled; the daemon was kept alive so the session can still be closed or inspected.', + ); + assert.equal( + resolveRequestTimeoutHint({ + remote: false, + resetDaemon: true, + command: 'open', + appleCleanupEvidence: false, + }), + 'Retry with --debug and check daemon diagnostics logs. The daemon was reset after the timeout.', + ); + assert.equal( + resolveRequestTimeoutHint({ + remote: false, + resetDaemon: false, + command: 'snapshot', + appleCleanupEvidence: false, + }), + 'Retry with --debug and check daemon diagnostics logs. The timed-out snapshot request was canceled; the daemon was kept alive so the session can still be closed or inspected.', + ); + + // Remote requests were never Apple-specific and stay evidence-independent. + assert.equal( + resolveRequestTimeoutHint({ + remote: true, + resetDaemon: false, + command: 'press', + appleCleanupEvidence: false, + }), + 'Retry with --debug and verify the remote daemon URL, auth token, and remote host logs.', + ); +}); + +test('a timed-out remote recording names the retry that returns the export', () => { + assert.equal( + resolveRequestTimeoutHint({ + remote: true, + resetDaemon: false, + command: 'record', + appleCleanupEvidence: false, + action: 'stop', + session: 'recording', + }), + 'The remote daemon is still exporting the recording. Run agent-device record stop --session recording again to wait for that export and receive the completed recording.', + ); + assert.equal( + resolveRequestTimeoutHint({ + remote: true, + resetDaemon: false, + command: 'record', + appleCleanupEvidence: false, + action: 'stop', + }), + 'The remote daemon is still exporting the recording. Run agent-device record stop again to wait for that export and receive the completed recording.', + ); + // A local timeout resets the daemon mid-export, so no keep-exporting promise is made. + assert.equal( + resolveRequestTimeoutHint({ + remote: false, + resetDaemon: true, + command: 'record', + appleCleanupEvidence: false, + action: 'stop', + session: 'recording', + }), + 'Retry with --debug and check daemon diagnostics logs. The daemon was reset after the timeout.', + ); + // `record start` runs no export, so it keeps the generic remote wording. + assert.equal( + resolveRequestTimeoutHint({ + remote: true, + resetDaemon: false, + command: 'record', + appleCleanupEvidence: false, + action: 'start', + session: 'recording', + }), + 'Retry with --debug and verify the remote daemon URL, auth token, and remote host logs.', + ); +}); diff --git a/src/daemon-client/__tests__/daemon-client.test.ts b/src/daemon-client/__tests__/daemon-client.test.ts index 15fa4c1d9c..1b6c2dcd3c 100644 --- a/src/daemon-client/__tests__/daemon-client.test.ts +++ b/src/daemon-client/__tests__/daemon-client.test.ts @@ -27,10 +27,7 @@ import { } from '../daemon-client-metadata.ts'; import { canConnectSocket } from '../daemon-client-transport.ts'; import { DAEMON_RPC_PROTOCOL_VERSION } from '@agent-device/contracts/daemon-http'; -import { - resolveRequestTimeoutHint, - shouldResetDaemonAfterRequestTimeout, -} from '../daemon-client-timeout.ts'; +import { shouldResetDaemonAfterRequestTimeout } from '../daemon-client-timeout.ts'; import { resolveDaemonPaths } from '../../daemon/config.ts'; import { stopProcessForTakeover } from '../../daemon/daemon-process.ts'; import { findProjectRoot, readVersion } from '@agent-device/host-kit/version'; @@ -251,97 +248,6 @@ test('read-only polling command timeouts preserve the daemon like snapshot', () assert.equal(shouldResetDaemonAfterRequestTimeout('open'), true); }); -test('request timeout hint only names Apple runner cleanup on actual evidence', () => { - // Before this change, handleRequestTimeout emitted Apple-specific hint - // wording for EVERY local timeout, regardless of the request's - // --platform. That was misleading for Android/web/Harmony sessions, which - // never had any Apple runner work to abort. - // - // The fix is evidence-based, not platform-guess-based: - // `appleCleanupEvidence` is true only when the request declared an - // AFFIRMATIVELY Apple platform (apple/ios/macos) or the pkill cleanup - // itself terminated a matching process — never from an undeclared or - // declared-non-Apple platform alone. (Why not trust the declared platform - // directly: it is not authoritative for session-bound execution — see - // `handleRequestTimeout`'s comment and the production-seam coverage in - // daemon-client-timeout-route.test.ts for the cleanup-eligibility half of - // this contract that this pure formatter test cannot prove.) - - // appleCleanupEvidence: true keeps the exact historical wording — nothing - // regresses for the true-Apple case. - assert.equal( - resolveRequestTimeoutHint({ - remote: false, - resetDaemon: false, - command: 'press', - appleCleanupEvidence: true, - }), - 'Retry with --debug and check daemon diagnostics logs. The timed-out press request was canceled and Apple runner work was aborted when detected; the daemon was kept alive so the session can still be closed or inspected.', - ); - assert.equal( - resolveRequestTimeoutHint({ - remote: false, - resetDaemon: true, - command: 'open', - appleCleanupEvidence: true, - }), - 'Retry with --debug and check daemon diagnostics logs. Timed-out Apple runner xcodebuild processes were terminated when detected.', - ); - assert.equal( - resolveRequestTimeoutHint({ - remote: false, - resetDaemon: false, - command: 'snapshot', - appleCleanupEvidence: true, - }), - 'Retry with --debug and check daemon diagnostics logs. The timed-out snapshot request was canceled and Apple runner work was aborted when detected; the daemon was kept alive so the session can still be closed or inspected. If this was the first Apple-platform snapshot on the device, run agent-device prepare ios-runner with the same --platform before snapshot/test so runner startup is handled explicitly.', - ); - - // appleCleanupEvidence: false — no Apple-runner claim in any branch, and - // the Apple-only iOS-prepare follow-up drops entirely. This is the - // motivating fix: it fires equally whether the platform was declared - // non-Apple OR left undeclared (the common session-bound case), because - // neither is Apple evidence on its own. - assert.equal( - resolveRequestTimeoutHint({ - remote: false, - resetDaemon: false, - command: 'press', - appleCleanupEvidence: false, - }), - 'Retry with --debug and check daemon diagnostics logs. The timed-out press request was canceled; the daemon was kept alive so the session can still be closed or inspected.', - ); - assert.equal( - resolveRequestTimeoutHint({ - remote: false, - resetDaemon: true, - command: 'open', - appleCleanupEvidence: false, - }), - 'Retry with --debug and check daemon diagnostics logs. The daemon was reset after the timeout.', - ); - assert.equal( - resolveRequestTimeoutHint({ - remote: false, - resetDaemon: false, - command: 'snapshot', - appleCleanupEvidence: false, - }), - 'Retry with --debug and check daemon diagnostics logs. The timed-out snapshot request was canceled; the daemon was kept alive so the session can still be closed or inspected.', - ); - - // Remote requests were never Apple-specific and stay evidence-independent. - assert.equal( - resolveRequestTimeoutHint({ - remote: true, - resetDaemon: false, - command: 'press', - appleCleanupEvidence: false, - }), - 'Retry with --debug and verify the remote daemon URL, auth token, and remote host logs.', - ); -}); - test('cleanupFailedDaemonStartupMetadata removes partial startup metadata', async () => { const stateDir = mkdtempForTestSync('agent-device-daemon-cleanup-'); const paths = resolveDaemonPaths(stateDir); diff --git a/src/daemon-client/daemon-client-timeout.ts b/src/daemon-client/daemon-client-timeout.ts index 6926be1449..0e2e624237 100644 --- a/src/daemon-client/daemon-client-timeout.ts +++ b/src/daemon-client/daemon-client-timeout.ts @@ -37,14 +37,21 @@ function isAffirmativelyApplePlatform(platform: PlatformSelector | undefined): b } export function handleRequestTimeout( - info: DaemonInfo, - statePaths: DaemonPaths, - requestId: string | undefined, - command: string | undefined, - remote: boolean, - timeoutMs: number, - platform: PlatformSelector | undefined, + params: Readonly<{ + info: DaemonInfo; + statePaths: DaemonPaths; + remote: boolean; + timeoutMs: number; + requestId: string | undefined; + command: string | undefined; + platform: PlatformSelector | undefined; + /** Named together so the recovery hint cannot be assembled from a swapped session and action. */ + session?: string; + action?: string; + }>, ): AppError { + const { info, statePaths, remote, timeoutMs, requestId, command, platform, session, action } = + params; // Cleanup eligibility stays UNCONDITIONAL for every local (non-remote) // timeout, on purpose: the request's declared --platform is not // authoritative for session-bound execution. An existing session's real @@ -87,7 +94,14 @@ export function handleRequestTimeout( return new AppError('COMMAND_FAILED', 'Daemon request timed out', { timeoutMs, requestId, - hint: resolveRequestTimeoutHint({ remote, resetDaemon, command, appleCleanupEvidence }), + hint: resolveRequestTimeoutHint({ + remote, + resetDaemon, + command, + appleCleanupEvidence, + session, + action, + }), }); } @@ -112,9 +126,20 @@ export function resolveRequestTimeoutHint(params: { resetDaemon: boolean; command: string | undefined; appleCleanupEvidence: boolean; + /** The request's first positional, for commands whose recovery depends on which action ran. */ + action?: string; + session?: string; }): string { - const { remote, resetDaemon, command, appleCleanupEvidence } = params; + const { remote, resetDaemon, command, appleCleanupEvidence, session, action } = params; if (remote) { + // A remote daemon survives this client window, so a `record stop` that ran out of time is still + // exporting there and its finished file stays retrievable by asking again. A local timeout + // resets the daemon mid-export, where that promise would be false. + if (command === PUBLIC_COMMANDS.record && action === 'stop') { + return `The remote daemon is still exporting the recording. Run agent-device record stop${ + session ? ` --session ${session}` : '' + } again to wait for that export and receive the completed recording.`; + } return 'Retry with --debug and verify the remote daemon URL, auth token, and remote host logs.'; } if (!resetDaemon) { diff --git a/src/daemon-client/daemon-client-transport.ts b/src/daemon-client/daemon-client-transport.ts index 8b81e06194..a927fd886d 100644 --- a/src/daemon-client/daemon-client-transport.ts +++ b/src/daemon-client/daemon-client-transport.ts @@ -336,15 +336,11 @@ async function sendSocketRequest( settled = true; socket.destroy(); reject( - handleRequestTimeout( + handleRequestTimeout({ info, statePaths, - req.meta?.requestId, - req.command, - false, - timeoutMs, - req.flags?.platform, - ), + ...timeoutRequestContext(req, false, timeoutMs), + }), ); }, timeoutMs) : undefined; @@ -379,6 +375,26 @@ async function sendSocketRequest( }); } +// The fields a timed-out request is described by, read once so a socket and an HTTP timeout cannot +// describe the same request differently. +type TimeoutRequestFields = Omit[0], 'info' | 'statePaths'>; + +function timeoutRequestContext( + req: DaemonRequest, + remote: boolean, + timeoutMs: number, +): TimeoutRequestFields { + return { + remote, + timeoutMs, + requestId: req.meta?.requestId, + command: req.command, + platform: req.flags?.platform, + session: req.session, + action: req.positionals?.[0], + }; +} + async function sendHttpRequest( info: DaemonInfo, req: DaemonRequest, @@ -463,15 +479,11 @@ async function sendHttpRequest( ? setTimeout(() => { request.destroy(); reject( - handleRequestTimeout( + handleRequestTimeout({ info, statePaths, - req.meta?.requestId, - req.command, - remote, - timeoutMs, - req.flags?.platform, - ), + ...timeoutRequestContext(req, remote, timeoutMs), + }), ); }, timeoutMs) : undefined; diff --git a/src/daemon/__tests__/screen-recording-stop-recovery.test.ts b/src/daemon/__tests__/screen-recording-stop-recovery.test.ts new file mode 100644 index 0000000000..191f20d05a --- /dev/null +++ b/src/daemon/__tests__/screen-recording-stop-recovery.test.ts @@ -0,0 +1,206 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { expect, test } from 'vitest'; +import type { JsonObject } from '@agent-device/contracts/client'; +import { localRuntimeOwner } from '@agent-device/contracts/platform-runtime'; +import type { ScreenRecordingCompletion } from '@agent-device/contracts/screen-recording-runtime'; +import { createDurableResourceEnvelope } from '@agent-device/capture-kit'; +import type { DeviceInfo } from '@agent-device/kernel/device'; +import { deviceIdentity } from '@agent-device/kernel/device'; +import { mkdtempForTestSync } from '../../__tests__/test-utils/tmp-dir.ts'; +import { makeSessionStore } from '../../__tests__/test-utils/store-factory.ts'; +import { screenRecordingResourceStore } from '../screen-recording-resource-store.ts'; +import { encodeScreenRecordingCompletionMetadata } from '../screen-recording-session-resource.ts'; +import { + resolveScreenRecordingStopRecovery, + screenRecordingManifestIsTerminal, +} from '../screen-recording-stop-recovery.ts'; +import type { SessionStore } from '../session-store.ts'; + +const SESSION_NAME = 'recording'; +const SESSION_DEVICE: DeviceInfo = { + platform: 'android', + id: 'emulator-5554', + name: 'Pixel', + kind: 'emulator', +}; + +test('a completed manifest with no surviving video serves nothing', async () => { + const harness = makeHarness(); + await completeRecording(harness); + fs.rmSync(harness.videoPath); + + expect(resolveScreenRecordingStopRecovery({ ...harness.params, device: SESSION_DEVICE })).toEqual( + { kind: 'none' }, + ); +}); + +test('a completed manifest hands back the whole stop response it stored', async () => { + const harness = makeHarness(); + const completion = fullCompletion(harness.videoPath); + fs.writeFileSync(harness.videoPath, 'mp4'); + writeManifest(harness, encodeScreenRecordingCompletionMetadata(completion)); + + expect(resolveScreenRecordingStopRecovery({ ...harness.params, device: SESSION_DEVICE })).toEqual( + { kind: 'completed', completion }, + ); + expect(screenRecordingManifestIsTerminal(harness.params)).toBe(true); +}); + +test('a completed manifest whose stored response is damaged serves nothing', async () => { + const harness = makeHarness(); + const stored = encodeScreenRecordingCompletionMetadata(fullCompletion(harness.videoPath)); + fs.writeFileSync(harness.videoPath, 'mp4'); + + const damages: JsonObject[] = [ + { outPath: 0 }, + { clientOutPath: '' }, + { completedAt: 'later' }, + { chunks: [{ index: 'first', path: '/daemon/capture-0.mp4' }] }, + { activeSessionApp: { bundleId: '' } }, + ]; + + for (const damaged of damages) { + writeManifest(harness, withStoredCompletion(stored, damaged)); + expect( + resolveScreenRecordingStopRecovery({ ...harness.params, device: SESSION_DEVICE }), + ).toEqual({ kind: 'none' }); + } +}); + +test('a completed manifest with no completion metadata serves nothing', async () => { + const harness = makeHarness(); + await completeRecording(harness); + writeManifest(harness, { phase: 'completed' }); + + expect(resolveScreenRecordingStopRecovery({ ...harness.params, device: SESSION_DEVICE })).toEqual( + { kind: 'none' }, + ); +}); + +test('a completed manifest owned by another session is refused', async () => { + const harness = makeHarness(); + await completeRecording(harness); + writeManifest(harness, undefined, { sessionId: 'other-session' }); + + expect(() => + resolveScreenRecordingStopRecovery({ ...harness.params, device: SESSION_DEVICE }), + ).toThrowError(expect.objectContaining({ details: { reason: 'runtime-contract-invalid' } })); +}); + +test('a completed manifest bound to another device is refused', async () => { + const harness = makeHarness(); + await completeRecording(harness); + writeManifest(harness, undefined, { + device: deviceIdentity({ ...SESSION_DEVICE, id: 'emulator-5556' }), + }); + + expect(() => + resolveScreenRecordingStopRecovery({ ...harness.params, device: SESSION_DEVICE }), + ).toThrowError(expect.objectContaining({ details: { reason: 'runtime-contract-invalid' } })); +}); + +test('an open manifest stays available for exact-owner recovery', async () => { + const harness = makeHarness(); + writeManifest(harness, { phase: 'active' }, { lifecycle: 'open' }); + + expect(resolveScreenRecordingStopRecovery({ ...harness.params, device: SESSION_DEVICE })).toEqual( + { kind: 'open', resourcePath: manifestPath(harness.sessionStore) }, + ); + expect(screenRecordingManifestIsTerminal(harness.params)).toBe(false); +}); + +test('a session with no recording manifest has nothing to recover', () => { + const harness = makeHarness(); + + expect(resolveScreenRecordingStopRecovery({ ...harness.params, device: SESSION_DEVICE })).toEqual( + { kind: 'none' }, + ); + expect(screenRecordingManifestIsTerminal(harness.params)).toBe(false); +}); + +function fullCompletion(outPath: string): ScreenRecordingCompletion { + return { + backend: 'simctl', + outPath, + startedAt: 1, + completedAt: 4, + scope: 'app', + showTouches: true, + recordOnlySession: false, + clientOutPath: '/workspace/capture.mp4', + telemetryPath: '/workspace/capture.gesture-telemetry.json', + warning: 'recording was truncated at the platform limit', + overlayWarning: 'touch overlay burn-in is only available on macOS hosts', + activeSessionApp: { bundleId: 'dev.example.app', name: 'Example' }, + chunks: [ + { index: 0, path: '/daemon/capture-0.mp4', clientOutPath: '/workspace/capture-0.mp4' }, + { index: 1, path: '/daemon/capture-1.mp4' }, + ], + }; +} + +function withStoredCompletion(metadata: JsonObject, patch: JsonObject): JsonObject { + const completion = metadata.completion as JsonObject; + return { ...metadata, completion: { ...completion, ...patch } }; +} + +function makeHarness() { + const sessionStore = makeSessionStore('screen-recording-stop-recovery-'); + const outputDir = mkdtempForTestSync('screen-recording-stop-recovery-output-'); + return { + sessionStore, + videoPath: path.join(outputDir, 'capture.mp4'), + params: { sessionName: SESSION_NAME, sessionStore }, + }; +} + +type Harness = ReturnType; + +function manifestPath(sessionStore: SessionStore): string { + return screenRecordingResourceStore.resolvePath(sessionStore.resolveSessionDir(SESSION_NAME)); +} + +async function completeRecording( + harness: Harness, + optional: Readonly<{ clientOutPath?: string; telemetryPath?: string }> = {}, +) { + fs.writeFileSync(harness.videoPath, 'mp4'); + const completion = { + backend: 'adb screenrecord', + outPath: harness.videoPath, + startedAt: 1, + completedAt: 2, + scope: 'app' as const, + showTouches: true, + recordOnlySession: false, + ...(optional.clientOutPath ? { clientOutPath: optional.clientOutPath } : {}), + ...(optional.telemetryPath ? { telemetryPath: optional.telemetryPath } : {}), + }; + writeManifest(harness, encodeScreenRecordingCompletionMetadata(completion)); + return completion; +} + +function writeManifest( + harness: Harness, + metadata: JsonObject | undefined, + overrides: { + sessionId?: string; + device?: ReturnType; + lifecycle?: 'open' | 'completed'; + } = {}, +): void { + screenRecordingResourceStore.write( + manifestPath(harness.sessionStore), + createDurableResourceEnvelope({ + resourceKind: 'screen-recording', + sessionId: overrides.sessionId ?? SESSION_NAME, + device: overrides.device ?? deviceIdentity(SESSION_DEVICE), + owner: localRuntimeOwner(SESSION_DEVICE.platform), + fence: { token: 'screen-recording-fence', generation: 1 }, + lifecycle: overrides.lifecycle ?? 'completed', + descriptor: { version: 1, body: { recordingId: 'recording-id' } }, + ...(metadata === undefined ? {} : { metadata }), + }), + ); +} diff --git a/src/daemon/durable-capture-resource.ts b/src/daemon/durable-capture-resource.ts index c231c4e335..c49559b4d2 100644 --- a/src/daemon/durable-capture-resource.ts +++ b/src/daemon/durable-capture-resource.ts @@ -44,7 +44,7 @@ export function createDurableCaptureResource< H extends LiveResourceHandle, C, >(definition: DurableCaptureResourceDefinition) { - const resourcePath = ( + const sessionResourcePath = ( sessionStore: DurableCaptureSessionStore, sessionName: string, ): string => definition.store.resolvePath(sessionStore.resolveSessionDir(sessionName)); @@ -58,6 +58,8 @@ export function createDurableCaptureResource< return Object.freeze({ store: definition.store, + /** Where this session's record for this resource lives. */ + resourcePath: sessionResourcePath, createNextFence(params: { admissionLedger: DurableCaptureAdmissionLedger; resourcePath: string; @@ -75,7 +77,7 @@ export function createDurableCaptureResource< else params.admissionLedger.blockUndurableCleanup(device, outcome.reason); }, }, - resourcePath(params.sessionStore, params.sessionName), + sessionResourcePath(params.sessionStore, params.sessionName), ); }, finishLive(params: { @@ -86,7 +88,7 @@ export function createDurableCaptureResource< return finishLiveDurableCapture( definition, params, - resourcePath(params.sessionStore, params.sessionName), + sessionResourcePath(params.sessionStore, params.sessionName), ); }, finishRecovered(params: FinishRecoveredDurableCaptureParams): Promise { diff --git a/src/daemon/handlers/__tests__/record-runtime-stop-recovery.test.ts b/src/daemon/handlers/__tests__/record-runtime-stop-recovery.test.ts index f790da01c6..113d8502e4 100644 --- a/src/daemon/handlers/__tests__/record-runtime-stop-recovery.test.ts +++ b/src/daemon/handlers/__tests__/record-runtime-stop-recovery.test.ts @@ -1,3 +1,4 @@ +import fs from 'node:fs'; import path from 'node:path'; import { expect, test } from 'vitest'; import { mkdtempForTestSync } from '../../../__tests__/test-utils/tmp-dir.ts'; @@ -123,3 +124,76 @@ test('record stop rejects a cross-session recovery manifest before exact-owner b }); expect(harness.runtime.bindExactDeviceCalls).not.toHaveBeenCalled(); }); + +test('record stop returns the export whose response never reached the caller', async () => { + const harness = makeRecordRuntimeHarness('record-runtime-replayed-stop-'); + const outPath = writeRecording('record-runtime-replayed-stop-output-'); + await harness.run(['start', outPath]); + await harness.run(['stop']); + + const recovered = await harness.run(['stop']); + + expect(recovered).toMatchObject({ + ok: true, + data: { recording: 'stopped', outPath, recordingBackend: 'adb screenrecord' }, + }); + expect(harness.runtime.finish).toHaveBeenCalledOnce(); + expect(harness.runtime.bindExactDeviceCalls).not.toHaveBeenCalled(); +}); + +test('a recovered stop keeps the caller-side output path that makes it downloadable', async () => { + const harness = makeRecordRuntimeHarness('record-runtime-replayed-remote-stop-'); + const cwd = mkdtempForTestSync('record-runtime-replayed-remote-stop-output-'); + const outPath = path.join(cwd, 'capture.mp4'); + fs.writeFileSync(outPath, 'mp4'); + await harness.run(['start', outPath], { + cwd, + clientArtifactPaths: { outPath: '/client/capture.mp4' }, + }); + await harness.run(['stop'], { cwd }); + + const recovered = await harness.run(['stop'], { cwd }); + + expect(recovered).toMatchObject({ + ok: true, + data: { + recording: 'stopped', + outPath, + artifacts: [{ field: 'outPath', path: outPath, localPath: '/client/capture.mp4' }], + }, + }); +}); + +test('a recovered stop does not record a second session stop action', async () => { + const harness = makeRecordRuntimeHarness('record-runtime-replayed-stop-action-'); + const outPath = writeRecording('record-runtime-replayed-stop-action-output-'); + await harness.run(['start', outPath]); + await harness.run(['stop']); + + await harness.run(['stop']); + + const actions = harness.sessionStore.get(harness.sessionName)?.actions ?? []; + expect(actions.filter((action) => action.positionals[0] === 'stop')).toHaveLength(1); +}); + +test('record stop reports no active recording once a completed export is gone', async () => { + const harness = makeRecordRuntimeHarness('record-runtime-deleted-stop-'); + const outPath = writeRecording('record-runtime-deleted-stop-output-'); + await harness.run(['start', outPath]); + await harness.run(['stop']); + fs.rmSync(outPath); + + const recovered = await harness.run(['stop']); + + expect(recovered).toMatchObject({ + ok: false, + error: { code: 'INVALID_ARGS', message: 'no active recording' }, + }); + expect(harness.runtime.bindExactDeviceCalls).not.toHaveBeenCalled(); +}); + +function writeRecording(prefix: string): string { + const outPath = path.join(mkdtempForTestSync(prefix), 'capture.mp4'); + fs.writeFileSync(outPath, 'mp4'); + return outPath; +} diff --git a/src/daemon/handlers/__tests__/record-runtime.fixtures.ts b/src/daemon/handlers/__tests__/record-runtime.fixtures.ts index fc29d001c6..f191c1c1d6 100644 --- a/src/daemon/handlers/__tests__/record-runtime.fixtures.ts +++ b/src/daemon/handlers/__tests__/record-runtime.fixtures.ts @@ -10,6 +10,7 @@ import { } from '@agent-device/contracts/platform-runtime'; import type { PlatformRuntimeOperations } from '@agent-device/contracts/platform-runtime-operations'; import type { ScreenRecordingLiveHandle } from '@agent-device/contracts/screen-recording-runtime'; +import type { DaemonRequest } from '../../daemon-request.ts'; import { unavailableDeploymentSnapshotAndShutdownOperationFacts } from '../../../__tests__/test-utils/runtime-operation-facts.ts'; import { createDurableResourceEnvelope } from '@agent-device/capture-kit'; import { makeSessionStore } from '../../../__tests__/test-utils/store-factory.ts'; @@ -70,7 +71,7 @@ export function makeRecordRuntimeHarness( sessionName, sessionStore, runtime, - run: (positionals: string[], meta?: { cwd: string }) => + run: (positionals: string[], meta?: DaemonRequest['meta']) => handleRecordCommand({ ...common, req: { @@ -104,10 +105,12 @@ export function recordingResourcePath( function makeRuntime(session: SessionState, options: RuntimeOptions = {}) { const owner = localRuntimeOwner(session.device.platform); let currentOutPath = ''; + let currentClientOutPath: string | undefined; const handle: ScreenRecordingLiveHandle = { inspect: () => ({ backend: 'adb screenrecord', outPath: currentOutPath, + ...(currentClientOutPath ? { clientOutPath: currentClientOutPath } : {}), startedAt: 1, scope: 'app', showTouches: true, @@ -125,6 +128,7 @@ function makeRuntime(session: SessionState, options: RuntimeOptions = {}) { result: { backend: 'adb screenrecord', outPath: currentOutPath, + ...(currentClientOutPath ? { clientOutPath: currentClientOutPath } : {}), startedAt: 1, completedAt: 2, scope: 'app' as const, @@ -139,6 +143,7 @@ function makeRuntime(session: SessionState, options: RuntimeOptions = {}) { const start = vi.fn( async (input: Parameters[0]) => { currentOutPath = input.outputPath; + currentClientOutPath = input.clientOutputPath; return { pendingHandle: new PendingTransferGuard(handle), envelope: createDurableResourceEnvelope({ diff --git a/src/daemon/handlers/record-runtime.ts b/src/daemon/handlers/record-runtime.ts index 1d6aac5966..684cab806e 100644 --- a/src/daemon/handlers/record-runtime.ts +++ b/src/daemon/handlers/record-runtime.ts @@ -1,6 +1,9 @@ import path from 'node:path'; import type { PlatformRequestScope } from '@agent-device/contracts/platform-runtime-host'; -import type { ScreenRecordingStartInput } from '@agent-device/contracts/screen-recording-runtime'; +import type { + ScreenRecordingCompletion, + ScreenRecordingStartInput, +} from '@agent-device/contracts/screen-recording-runtime'; import { resolveScreenRecordingRuntimePlan, screenRecordingAdmissionUse, @@ -8,7 +11,6 @@ import { screenRecordingStartUse, } from '@agent-device/contracts/screen-recording-runtime-plan'; import { isWholeScreenRecordingScope } from '@agent-device/contracts/recording'; -import { deviceIdentity, sameDeviceIdentity } from '@agent-device/kernel/device'; import { AppError, normalizeError } from '@agent-device/kernel/errors'; import { resolveTargetDevice } from '@agent-device/device-selection/dispatch-resolve'; import { ensureBoundDeviceReady } from '../request-runtime-binding.ts'; @@ -20,6 +22,10 @@ import { screenRecordingDurableResource, } from '../screen-recording-session-resource.ts'; import { createScreenRecordingRecoveryControl } from '../screen-recording-resource-recovery.ts'; +import { + resolveScreenRecordingStopRecovery, + screenRecordingManifestIsTerminal, +} from '../screen-recording-stop-recovery.ts'; import { resolveSessionScope } from '../session-routing.ts'; import type { SessionStore } from '../session-store.ts'; import type { BindDeviceRuntime, BindExactDeviceRuntime } from '../request-runtime-binding.ts'; @@ -39,6 +45,15 @@ import { buildRecordingUnsupportedResponse, } from './record-runtime-response.ts'; +/** + * A stop either finishes the live export or reports one the daemon already finished. Only the + * first owes a session action: the request that terminalized the recording already recorded it. + */ +type ScreenRecordingStop = Readonly<{ + completion: ScreenRecordingCompletion; + recordsSessionAction: boolean; +}>; + export type RecordRuntimeHandlerParams = Readonly<{ req: DaemonRequest; sessionName: string; @@ -154,8 +169,9 @@ async function startRecording( } function prepareRecordingStart(params: RecordRuntimeHandlerParams, session: SessionState) { - const resourcePath = screenRecordingDurableResource.store.resolvePath( - params.sessionStore.resolveSessionDir(params.sessionName), + const resourcePath = screenRecordingDurableResource.resourcePath( + params.sessionStore, + params.sessionName, ); return { fence: screenRecordingDurableResource.createNextFence({ @@ -206,29 +222,35 @@ async function stopRecording( kind: 'stop-live' | 'stop-recovery', needsReadiness: boolean, ): Promise { - let completion; + let stopped: ScreenRecordingStop; try { - completion = + stopped = kind === 'stop-live' - ? await finishLiveScreenRecording({ - session, - sessionName: params.sessionName, - sessionStore: params.sessionStore, - }) + ? { + completion: await finishLiveScreenRecording({ + session, + sessionName: params.sessionName, + sessionStore: params.sessionStore, + }), + recordsSessionAction: true, + } : await finishRecovered(params, session, needsReadiness); } catch (error) { deleteTerminalRecordOnlySession(params, session); throw error; } + const completion = stopped.completion; const response = buildRecordingStopResponse(completion); - recordSessionAction(params.sessionStore, session, params.req, params.req.command, { - action: 'stop', - outPath: completion.outPath, - ...(completion.clientOutPath - ? { requestedFileName: path.basename(completion.clientOutPath) } - : {}), - showTouches: completion.showTouches, - }); + if (stopped.recordsSessionAction) { + recordSessionAction(params.sessionStore, session, params.req, params.req.command, { + action: 'stop', + outPath: completion.outPath, + ...(completion.clientOutPath + ? { requestedFileName: path.basename(completion.clientOutPath) } + : {}), + showTouches: completion.showTouches, + }); + } if (session.recordOnlySession) params.sessionStore.delete(params.sessionName); return response; } @@ -238,43 +260,27 @@ function deleteTerminalRecordOnlySession( session: SessionState, ): void { if (!session.recordOnlySession) return; - const resourcePath = screenRecordingDurableResource.store.resolvePath( - params.sessionStore.resolveSessionDir(params.sessionName), - ); - const record = screenRecordingDurableResource.store.read(resourcePath); - if (record.status === 'decoded' && record.envelope.lifecycle === 'completed') { - params.sessionStore.delete(params.sessionName); - } + if (screenRecordingManifestIsTerminal(params)) params.sessionStore.delete(params.sessionName); } async function finishRecovered( params: RecordRuntimeHandlerParams, session: SessionState, needsReadiness: boolean, -) { - const resourcePath = screenRecordingDurableResource.store.resolvePath( - params.sessionStore.resolveSessionDir(params.sessionName), - ); - const record = screenRecordingDurableResource.store.read(resourcePath); - if (record.status !== 'decoded' || record.envelope.lifecycle !== 'open') { - throw new AppError('INVALID_ARGS', 'no active recording'); - } - if (record.envelope.sessionId !== params.sessionName) { - throw new AppError( - 'COMMAND_FAILED', - 'Screen recording recovery record does not belong to the requested session', - { reason: 'runtime-contract-invalid' }, - ); +): Promise { + const recovery = resolveScreenRecordingStopRecovery({ + sessionName: params.sessionName, + sessionStore: params.sessionStore, + device: session.device, + }); + if (recovery.kind === 'completed') { + return { completion: recovery.completion, recordsSessionAction: false }; } - if (!sameDeviceIdentity(record.envelope.device, deviceIdentity(session.device))) { - throw new AppError( - 'COMMAND_FAILED', - 'Screen recording recovery device does not match the selected device', - { reason: 'runtime-contract-invalid' }, - ); + if (recovery.kind === 'none') { + throw new AppError('INVALID_ARGS', 'no active recording'); } - return await finishRecoveredScreenRecording({ - resourcePath, + const completion = await finishRecoveredScreenRecording({ + resourcePath: recovery.resourcePath, scope: params.requestScope, acquireControl: async (envelope, recoveryScope) => { const runtime = await params.bindExactDevice( @@ -288,6 +294,7 @@ async function finishRecovered( return createScreenRecordingRecoveryControl({ runtime, dispose: async () => {} }); }, }); + return { completion, recordsSessionAction: true }; } function createRecordOnlySession( diff --git a/src/daemon/screen-recording-session-resource.ts b/src/daemon/screen-recording-session-resource.ts index 0b4c5129c4..b57a3ae2f9 100644 --- a/src/daemon/screen-recording-session-resource.ts +++ b/src/daemon/screen-recording-session-resource.ts @@ -1,3 +1,4 @@ +import type { JsonObject } from '@agent-device/contracts/client'; import type { DurableResourceEnvelope } from '@agent-device/contracts/durable-resource-envelope'; import type { PendingTransferGuard } from '@agent-device/contracts/async-lifecycle'; import type { PlatformRequestScope } from '@agent-device/contracts/platform-runtime-host'; @@ -6,9 +7,11 @@ import type { RuntimeOwnerRef, } from '@agent-device/contracts/platform-runtime'; import type { + ScreenRecordingChunk, ScreenRecordingCompletion, ScreenRecordingLiveHandle, } from '@agent-device/contracts/screen-recording-runtime'; +import type { RecordingAppIdentity } from '@agent-device/contracts/recording'; import type { DeviceInfo } from '@agent-device/kernel/device'; import type { DurableCaptureRecoveryControl } from '@agent-device/capture-kit/durable-capture'; import { createDurableCaptureResource } from './durable-capture-resource.ts'; @@ -29,15 +32,7 @@ export const screenRecordingDurableResource = createDurableCaptureResource< read: (session) => session.screenRecording, replace: (session, screenRecording) => ({ ...session, screenRecording }), }, - completionMetadata: (completion) => ({ - backend: completion.backend, - outputPath: completion.outPath, - startedAt: completion.startedAt, - completedAt: completion.completedAt, - scope: completion.scope, - showTouches: completion.showTouches, - recordOnlySession: completion.recordOnlySession, - }), + completionMetadata: encodeScreenRecordingCompletionMetadata, messages: { noActive: 'no active recording', cleanupPendingHint: @@ -68,6 +63,55 @@ export function finishLiveScreenRecording(params: { return screenRecordingDurableResource.finishLive(params); } +/** + * The manifest key holding a finished recording's stop response. The completion is stored as the one + * object `record stop` returned, so a replay cannot lose a field on its way through the manifest; + * `screen-recording-stop-recovery.ts` reads it back and serves it. + */ +export const SCREEN_RECORDING_COMPLETION_METADATA_KEY = 'completion'; + +export function encodeScreenRecordingCompletionMetadata( + completion: ScreenRecordingCompletion, +): JsonObject { + return { + [SCREEN_RECORDING_COMPLETION_METADATA_KEY]: { + backend: completion.backend, + outPath: completion.outPath, + startedAt: completion.startedAt, + completedAt: completion.completedAt, + scope: completion.scope, + showTouches: completion.showTouches, + recordOnlySession: completion.recordOnlySession, + ...(completion.clientOutPath === undefined + ? {} + : { clientOutPath: completion.clientOutPath }), + ...(completion.telemetryPath === undefined + ? {} + : { telemetryPath: completion.telemetryPath }), + ...(completion.warning === undefined ? {} : { warning: completion.warning }), + ...(completion.overlayWarning === undefined + ? {} + : { overlayWarning: completion.overlayWarning }), + ...(completion.activeSessionApp === undefined + ? {} + : { activeSessionApp: encodeAppIdentity(completion.activeSessionApp) }), + ...(completion.chunks === undefined ? {} : { chunks: completion.chunks.map(encodeChunk) }), + }, + }; +} + +function encodeAppIdentity(app: RecordingAppIdentity): JsonObject { + return { bundleId: app.bundleId, ...(app.name === undefined ? {} : { name: app.name }) }; +} + +function encodeChunk(chunk: ScreenRecordingChunk): JsonObject { + return { + index: chunk.index, + path: chunk.path, + ...(chunk.clientOutPath === undefined ? {} : { clientOutPath: chunk.clientOutPath }), + }; +} + export function finishRecoveredScreenRecording(params: { resourcePath: string; scope: PlatformRequestScope; diff --git a/src/daemon/screen-recording-stop-recovery.ts b/src/daemon/screen-recording-stop-recovery.ts new file mode 100644 index 0000000000..5457522ee5 --- /dev/null +++ b/src/daemon/screen-recording-stop-recovery.ts @@ -0,0 +1,177 @@ +import fs from 'node:fs'; +import type { JsonObject } from '@agent-device/contracts/client'; +import { isRecordingScope, type RecordingAppIdentity } from '@agent-device/contracts/recording'; +import type { + ScreenRecordingChunk, + ScreenRecordingCompletion, +} from '@agent-device/contracts/screen-recording-runtime'; +import type { DurableResourceEnvelope } from '@agent-device/contracts/durable-resource-envelope'; +import { deviceIdentity, sameDeviceIdentity, type DeviceInfo } from '@agent-device/kernel/device'; +import { AppError } from '@agent-device/kernel/errors'; +import { isRecord } from '@agent-device/kernel/record'; +import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; +import type { DurableCaptureResourceRecord } from '@agent-device/capture-kit/durable-capture'; +import { + SCREEN_RECORDING_COMPLETION_METADATA_KEY, + screenRecordingDurableResource, +} from './screen-recording-session-resource.ts'; +import type { SessionStore } from './session-store.ts'; + +/** + * What a `record stop` owes a session, decided from the durable recording manifest alone. + * + * A caller that stopped waiting while the daemon was still exporting leaves a `completed` manifest + * behind: the export exists on the daemon host, but its response never reached the caller. Serving + * that response from the manifest is what makes a retried `record stop` a supported recovery instead + * of a dead end. Only a response that is whole and whose file survived is served — a caller is never + * handed a path it cannot download. + */ +export type ScreenRecordingStopRecovery = + | Readonly<{ kind: 'completed'; completion: ScreenRecordingCompletion }> + | Readonly<{ kind: 'open'; resourcePath: string }> + | Readonly<{ kind: 'none' }>; + +const OPTIONAL_RESPONSE_FIELDS = [ + 'clientOutPath', + 'telemetryPath', + 'warning', + 'overlayWarning', +] as const; + +type ScreenRecordingManifestParams = Readonly<{ + sessionName: string; + sessionStore: SessionStore; +}>; + +export function resolveScreenRecordingStopRecovery( + params: ScreenRecordingManifestParams & Readonly<{ device: DeviceInfo }>, +): ScreenRecordingStopRecovery { + const { resourcePath, record } = readSessionManifest(params); + if (record.status !== 'decoded') return { kind: 'none' }; + assertManifestBelongsToRequest(params, record.envelope); + if (record.envelope.lifecycle !== 'completed') return { kind: 'open', resourcePath }; + const completion = readStoredCompletion(record.envelope.metadata); + if (completion === undefined) { + emitDiagnostic({ + level: 'warn', + phase: 'screen_recording_completed_manifest_unreplayable', + data: { resourcePath }, + }); + return { kind: 'none' }; + } + if (!fs.existsSync(completion.outPath)) return { kind: 'none' }; + return { kind: 'completed', completion }; +} + +/** Whether the session's manifest recorded a terminal recording, even one with no serveable file. */ +export function screenRecordingManifestIsTerminal(params: ScreenRecordingManifestParams): boolean { + const { record } = readSessionManifest(params); + return record.status === 'decoded' && record.envelope.lifecycle === 'completed'; +} + +function readSessionManifest(params: ScreenRecordingManifestParams): Readonly<{ + resourcePath: string; + record: DurableCaptureResourceRecord<'screen-recording'>; +}> { + const resourcePath = screenRecordingDurableResource.resourcePath( + params.sessionStore, + params.sessionName, + ); + return { resourcePath, record: screenRecordingDurableResource.store.read(resourcePath) }; +} + +function assertManifestBelongsToRequest( + params: Readonly<{ sessionName: string; device: DeviceInfo }>, + envelope: DurableResourceEnvelope<'screen-recording'>, +): void { + if (envelope.sessionId !== params.sessionName) { + throw new AppError( + 'COMMAND_FAILED', + 'Screen recording recovery record does not belong to the requested session', + { reason: 'runtime-contract-invalid' }, + ); + } + if (!sameDeviceIdentity(envelope.device, deviceIdentity(params.device))) { + throw new AppError( + 'COMMAND_FAILED', + 'Screen recording recovery device does not match the selected device', + { reason: 'runtime-contract-invalid' }, + ); + } +} + +/** + * The stop response the manifest stored, or nothing when it is not a whole one. The envelope store + * has already bounded this metadata to frozen plain JSON, so what recovery still has to establish is + * that the stored response answers correctly: the path it serves, the caller-side paths a remote + * download is named after, and the duration the response subtracts. + */ +function readStoredCompletion( + metadata: JsonObject | undefined, +): ScreenRecordingCompletion | undefined { + const stored = metadata?.[SCREEN_RECORDING_COMPLETION_METADATA_KEY]; + if (!isRecord(stored) || !isServedCompletion(stored) || !isWholeOptionalResponse(stored)) { + return undefined; + } + return stored as unknown as ScreenRecordingCompletion; +} + +/** The values a stop response computes on rather than repeats. */ +function isServedCompletion(stored: Record): boolean { + return ( + isNonEmptyString(stored.outPath) && + isNonEmptyString(stored.backend) && + isFiniteNumber(stored.startedAt) && + isFiniteNumber(stored.completedAt) && + isRecordingScope(stored.scope) && + typeof stored.showTouches === 'boolean' && + typeof stored.recordOnlySession === 'boolean' + ); +} + +/** Optional response fields the builder hands to `path.basename` must be whole when present. */ +function isWholeOptionalResponse(stored: Record): boolean { + return ( + OPTIONAL_RESPONSE_FIELDS.every((field) => isOptionalText(stored[field])) && + isOptionalAppIdentity(stored.activeSessionApp) && + isOptionalChunks(stored.chunks) + ); +} + +function isNonEmptyString(value: unknown): value is string { + return typeof value === 'string' && value.length > 0; +} + +function isOptionalText(value: unknown): value is string | undefined { + return value === undefined || isNonEmptyString(value); +} + +function isFiniteNumber(value: unknown): value is number { + return typeof value === 'number' && Number.isFinite(value); +} + +function isAppIdentity(value: unknown): value is RecordingAppIdentity { + if (!isRecord(value) || !isNonEmptyString(value.bundleId)) return false; + return isOptionalText(value.name); +} + +function isOptionalAppIdentity(value: unknown): value is RecordingAppIdentity | undefined { + return value === undefined || isAppIdentity(value); +} + +function isChunks(value: unknown): value is readonly ScreenRecordingChunk[] { + return ( + Array.isArray(value) && + value.every( + (chunk) => + isRecord(chunk) && + Number.isFinite(chunk.index) && + isNonEmptyString(chunk.path) && + isOptionalText(chunk.clientOutPath), + ) + ); +} + +function isOptionalChunks(value: unknown): value is readonly ScreenRecordingChunk[] | undefined { + return value === undefined || isChunks(value); +} diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 1f6f6e7f60..712695b56e 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -958,6 +958,7 @@ agent-device record stop # Stop active recording - On Linux or other non-macOS hosts, `record stop` still succeeds and returns the raw video plus telemetry sidecar, and includes `overlayWarning` when burn-in overlays were skipped. - On iOS simulators, a busy CoreSimulator host recording slot makes `record start` return non-retriable `DEVICE_IN_USE` with `details.reason: apple_simulator_recording_busy`. Use `record stop` in the session that owns the active recording. If a previous recorder died and no recording is active, ask the host operator to restart the CoreSimulator stream service before retrying. - Android uses `adb shell screenrecord`, which has a 180s platform limit. `record start` publishes a durable device manifest. Longer recordings are split into MP4 chunks while the daemon stays alive; after daemon restart, `record stop` recovers only manifest-owned chunks and warns when gesture overlay telemetry was lost. +- `record stop` is safe to repeat. When its request window ends while the daemon is still exporting — typical for a long touch-overlay burn-in on a remote daemon — the export keeps running there, and a second `record stop` in the same session returns that completed recording, including the caller-side output path, without starting another recording. A finished recording whose video file is already gone reports `no active recording`. **Session app logs (token-efficient debugging):** Logging is off by default in normal flows. Enable it on demand for debugging. Logs are written to a file so agents can grep instead of loading full output into context.