diff --git a/packages/capture-kit/src/recording/__tests__/overlay-script-paths.test.ts b/packages/capture-kit/src/recording/__tests__/overlay-script-paths.test.ts index 5051eff479..fb20b92967 100644 --- a/packages/capture-kit/src/recording/__tests__/overlay-script-paths.test.ts +++ b/packages/capture-kit/src/recording/__tests__/overlay-script-paths.test.ts @@ -12,7 +12,7 @@ vi.mock('@agent-device/host-kit/command', async () => { }), }; }); -vi.mock('../video.ts', () => ({ waitForPlayableVideo: vi.fn(async () => {}) })); +vi.mock('../video.ts', () => ({ waitForPlayableVideo: vi.fn(async () => true) })); vi.mock('../swift-cache.ts', () => ({ buildSwiftToolEnv: () => ({}), compileSwiftSourceFile: vi.fn(async () => '/bin/true'), diff --git a/packages/capture-kit/src/recording/__tests__/overlay.test.ts b/packages/capture-kit/src/recording/__tests__/overlay.test.ts index e42b12d359..6895e2dae2 100644 --- a/packages/capture-kit/src/recording/__tests__/overlay.test.ts +++ b/packages/capture-kit/src/recording/__tests__/overlay.test.ts @@ -25,7 +25,7 @@ vi.mock('@agent-device/host-kit/command', async (importOriginal) => { }); vi.mock('../video.ts', () => ({ - waitForPlayableVideo: vi.fn(async () => {}), + waitForPlayableVideo: vi.fn(async () => true), })); import { @@ -322,6 +322,27 @@ test('overlay keeps the plain failure when the export failed with budget to spar }); }); +test('overlay keeps the video as recorded when the export never plays within its budget', async () => { + const videoPath = path.join(tmpDir, 'recording.mp4'); + const telemetryPath = path.join(tmpDir, 'recording.gesture-telemetry.json'); + fs.writeFileSync(videoPath, 'original'); + fs.writeFileSync(telemetryPath, '{"events":[]}'); + vi.mocked(waitForPlayableVideo).mockResolvedValueOnce(false); + + await expect( + overlayRecordingTouches({ + host: createTestHost(), + videoPath, + telemetryPath, + deadline: startRecordingOverlayBudget(), + }), + ).rejects.toMatchObject({ + code: 'COMMAND_FAILED', + message: 'Failed to add touch overlays to the recording', + }); + expect(fs.readFileSync(videoPath, 'utf8')).toBe('original'); +}); + test('overlay keeps the video as recorded when verifying the export spent its budget', async () => { vi.useFakeTimers({ toFake: ['Date'] }); const videoPath = path.join(tmpDir, 'recording.mp4'); @@ -331,6 +352,7 @@ test('overlay keeps the video as recorded when verifying the export spent its bu const deadline = startRecordingOverlayBudget(); vi.mocked(waitForPlayableVideo).mockImplementationOnce(async () => { vi.setSystemTime(Date.now() + OVERLAY_BUDGET_MS); + return false; }); await expect( diff --git a/packages/capture-kit/src/recording/overlay.ts b/packages/capture-kit/src/recording/overlay.ts index 6f1d9f46b4..670c43b4a4 100644 --- a/packages/capture-kit/src/recording/overlay.ts +++ b/packages/capture-kit/src/recording/overlay.ts @@ -117,13 +117,9 @@ async function exportProcessedVideo(params: { exportOverran = deadline.remainingMs() <= HELPER_EXIT_GRACE_MS; throw error; } - await waitForPlayableVideo(outputPath, { host, deadline }); - if (deadline.isExpired()) { - exportOverran = true; - throw new AppError( - 'COMMAND_FAILED', - 'The composited video was not verified within the budget', - ); + if (!(await waitForPlayableVideo(outputPath, { host, deadline }))) { + exportOverran = deadline.isExpired(); + throw new AppError('COMMAND_FAILED', 'The composited video was not verified as playable'); } fs.renameSync(outputPath, videoPath); } catch (error) { diff --git a/packages/capture-kit/src/recording/swift-cache.test.ts b/packages/capture-kit/src/recording/swift-cache.test.ts index 5087846756..0503c34db5 100644 --- a/packages/capture-kit/src/recording/swift-cache.test.ts +++ b/packages/capture-kit/src/recording/swift-cache.test.ts @@ -3,6 +3,7 @@ import { createTestHost } from '../host-environment.fixtures.ts'; import fs from 'node:fs'; import path from 'node:path'; import { mkdtempForTestSync } from '../tmp-dir.fixtures.ts'; +import { createRequestCanceledError, isRequestCanceledError } from '@agent-device/kernel/errors'; vi.mock(import('@agent-device/host-kit/command'), async (importOriginal) => ({ ...(await importOriginal()), @@ -170,6 +171,62 @@ test('compileSwiftSourceText resolves a cache name with a long interior dash run expect(elapsedMs).toBeLessThan(1_000); }); +test('a canceled compile kills swiftc through its signal and leaves no temp output', async () => { + const cancel = new AbortController(); + mockRunCmd.mockImplementationOnce( + async (_cmd, _args, options) => + await new Promise((_, reject) => { + options?.signal?.addEventListener('abort', () => reject(createRequestCanceledError())); + }), + ); + + const compiled = compileSwiftSourceText({ + host: createTestHost(), + source: 'print(1)', + cacheName: 'canceled', + signal: cancel.signal, + }); + await vi.waitFor(() => expect(mockRunCmd).toHaveBeenCalledOnce()); + cancel.abort(); + + await expect(compiled).rejects.toSatisfy(isRequestCanceledError); + expect(mockRunCmd.mock.calls[0]![2]).toMatchObject({ + signal: cancel.signal, + kill: { signal: 'SIGINT' }, + }); + expect(fs.readdirSync(path.join(tmpDir, 'swift-cache', 'bin'))).toEqual([]); +}); + +test('a canceled wait on another process compile stops waiting and leaves its lock alone', async () => { + const compile = (signal?: AbortSignal) => + compileSwiftSourceText({ + host: createTestHost(), + source: 'print(1)', + cacheName: 'held-elsewhere', + signal, + }); + const executablePath = await compile(); + fs.rmSync(executablePath); + const lockDir = `${executablePath}.lock`; + fs.mkdirSync(lockDir); + vi.clearAllMocks(); + // The parent process stands in for another live process that holds the lock while it compiles. + fs.writeFileSync( + path.join(lockDir, 'owner.json'), + JSON.stringify({ pid: process.ppid, startTime: null, acquiredAtMs: Date.now() }), + ); + const cancel = new AbortController(); + + const compiled = compile(cancel.signal); + // Let the wait poll the held lock a few times before it is canceled. + await new Promise((resolve) => setTimeout(resolve, 100)); + cancel.abort(); + + await expect(compiled).rejects.toSatisfy(isRequestCanceledError); + expect(mockRunCmd).not.toHaveBeenCalled(); + expect(fs.existsSync(path.join(lockDir, 'owner.json'))).toBe(true); +}); + test('compileSwiftSourceText falls back to swift-helper when the cache name sanitizes to nothing', async () => { const executablePath = await compileSwiftSourceText({ host: createTestHost(), diff --git a/packages/capture-kit/src/recording/swift-cache.ts b/packages/capture-kit/src/recording/swift-cache.ts index 9c4ed0aad8..d2a7872b29 100644 --- a/packages/capture-kit/src/recording/swift-cache.ts +++ b/packages/capture-kit/src/recording/swift-cache.ts @@ -16,6 +16,9 @@ import { const SWIFT_CACHE_VERSION = '2'; const LOCK_RETRY_DELAY_MS = 25; +// swift-driver runs each frontend job in its own process group. It stops them on SIGINT, but +// SIGKILL or SIGTERM leaves them compiling with no parent. +const SWIFTC_STOP = { signal: 'SIGINT', graceMs: 2_000 } as const; const RECORDING_SCRIPT_SUBDIRECTORY = 'apple/runner/AgentDeviceRunner/RecordingScripts'; /** @@ -130,6 +133,8 @@ export async function compileSwiftSourceText(params: { source: string; cacheName: string; timeoutMs?: number; + /** Cancels the compile: kills `swiftc`, or stops waiting for another process's compile. */ + signal?: AbortSignal; }): Promise { const cacheName = sanitizeCacheName(params.cacheName); const key = hashParts([SWIFT_CACHE_VERSION, process.platform, process.arch, params.source]); @@ -143,6 +148,7 @@ export async function compileSwiftSourceText(params: { executablePath, sourceText: params.source, timeoutMs: params.timeoutMs, + signal: params.signal, }); return executablePath; } @@ -162,6 +168,7 @@ async function ensureSwiftExecutable(params: { executablePath: string; sourceText?: string; timeoutMs?: number; + signal?: AbortSignal; }): Promise { if (isExecutableFile(params.executablePath)) { return; @@ -171,7 +178,7 @@ async function ensureSwiftExecutable(params: { fs.mkdirSync(executableDir, { recursive: true }); const timeoutMs = params.timeoutMs ?? 120_000; await withProcessLock({ - acquire: () => acquireSwiftCacheLock(`${params.executablePath}.lock`, timeoutMs), + acquire: () => acquireSwiftCacheLock(`${params.executablePath}.lock`, timeoutMs, params.signal), task: async () => { // Another process may have published the executable while this one waited for the lock. if (isExecutableFile(params.executablePath)) { @@ -193,6 +200,8 @@ async function ensureSwiftExecutable(params: { } await runCmd('xcrun', ['swiftc', ...params.sourcePaths, '-o', tempExecutablePath], { timeoutMs, + signal: params.signal, + kill: SWIFTC_STOP, env: buildSwiftToolEnv(params.host.env), }); fs.renameSync(tempExecutablePath, params.executablePath); @@ -206,6 +215,7 @@ async function ensureSwiftExecutable(params: { async function acquireSwiftCacheLock( lockDir: string, timeoutMs: number, + signal: AbortSignal | undefined, ): Promise { try { return await acquireProcessLock({ @@ -217,6 +227,7 @@ async function acquireSwiftCacheLock( }, timeoutMs, pollMs: LOCK_RETRY_DELAY_MS, + signal, description: `Swift cache lock: ${lockDir} (${timeoutMs}ms)`, }); } catch (error) { diff --git a/packages/capture-kit/src/recording/video.test.ts b/packages/capture-kit/src/recording/video.test.ts index 00039aae51..0c8dd214c1 100644 --- a/packages/capture-kit/src/recording/video.test.ts +++ b/packages/capture-kit/src/recording/video.test.ts @@ -3,14 +3,16 @@ import { createTestHost } from '../host-environment.fixtures.ts'; import path from 'node:path'; import { beforeEach, expect, test, vi } from 'vitest'; import { runCmd } from '@agent-device/host-kit/command'; +import { AppError } from '@agent-device/kernel/errors'; import { likelyPlayableWebmContainer } from '../__tests__/test-utils/video-fixtures.ts'; import { mkdtempForTestSync } from '../tmp-dir.fixtures.ts'; import { mp4Atom, mp4MovieHeader } from './mp4.fixtures.ts'; import { Deadline } from '@agent-device/host-kit/retry'; import { compileSwiftSourceText } from './swift-cache.ts'; -import { isPlayableVideo, waitForPlayableVideo } from './video.ts'; +import { hasVideoContainer, isPlayableVideo, waitForPlayableVideo } from './video.ts'; -vi.mock('@agent-device/host-kit/command', () => ({ +vi.mock(import('@agent-device/host-kit/command'), async (importOriginal) => ({ + ...(await importOriginal()), runCmd: vi.fn(async () => ({ exitCode: 0, stdout: '', stderr: '' })), })); vi.mock('./swift-cache.ts', () => ({ @@ -69,6 +71,153 @@ test('each Swift cache root compiles its own validator, and a root reuses the on ).toEqual(['/cache/one', '/cache/two']); }); +test('callers share one validator compile, and each waits for it within its own budget', async () => { + // A cold compile on a loaded CI runner outlasted a fixed 30 s wait while record stop had most of + // its 70 s finalization budget left. A caller without a deadline that starts the compile first must + // not impose its 30 s wait on a concurrent caller that has a deadline. + const mp4 = writeFixture('shared-compile.mp4', Buffer.concat([ftyp, moov])); + const host = { env: { AGENT_DEVICE_SWIFT_CACHE_DIR: '/cache/shared' }, cwd: directory }; + await hasVideoContainer(mp4); + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }); + try { + vi.mocked(compileSwiftSourceText) + .mockClear() + .mockImplementationOnce( + () => new Promise((resolve) => setTimeout(() => resolve('/bin/true'), 45_000)), + ); + + const withoutDeadline = isPlayableVideo(mp4, host); + const withDeadline = isPlayableVideo(mp4, host, Deadline.fromTimeoutMs(70_000)); + const withoutDeadlineRejected = expect(withoutDeadline).rejects.toMatchObject({ + code: 'COMMAND_FAILED', + details: { + reason: 'video_validator_compile_timeout', + timeoutMs: 30_000, + retriable: true, + hint: expect.stringContaining('Run record stop again'), + }, + }); + await vi.advanceTimersByTimeAsync(45_000); + + await withoutDeadlineRejected; + await expect(withDeadline).resolves.toBe(true); + expect(compileSwiftSourceText).toHaveBeenCalledOnce(); + const [compile] = vi.mocked(compileSwiftSourceText).mock.calls[0]!; + expect(compile.timeoutMs).toBeUndefined(); + expect(compile.signal?.aborted).toBe(false); + } finally { + vi.useRealTimers(); + } +}); + +test('the last caller to stop waiting cancels an unfinished compile, and the next caller compiles again', async () => { + // Nothing owns swiftc once its requests end: a daemon that exits while it runs leaves it + // reparented to launchd, and the compile's own temp output behind. + const mp4 = writeFixture('cancel-compile.mp4', Buffer.concat([ftyp, moov])); + const host = { env: { AGENT_DEVICE_SWIFT_CACHE_DIR: '/cache/cancel' }, cwd: directory }; + await hasVideoContainer(mp4); + let compileSignal: AbortSignal | undefined; + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }); + try { + vi.mocked(compileSwiftSourceText) + .mockClear() + .mockImplementationOnce(async ({ signal }) => { + compileSignal = signal; + return await new Promise((_, reject) => { + signal?.addEventListener('abort', () => reject(signal.reason)); + }); + }); + + const timedOut = expect( + isPlayableVideo(mp4, host, Deadline.fromTimeoutMs(5_000)), + ).rejects.toMatchObject({ details: { reason: 'video_validator_compile_timeout' } }); + await vi.advanceTimersByTimeAsync(5_000); + await timedOut; + expect(compileSignal?.aborted).toBe(true); + } finally { + vi.useRealTimers(); + } + + await expect(isPlayableVideo(mp4, host)).resolves.toBe(true); + expect(compileSwiftSourceText).toHaveBeenCalledTimes(2); +}); + +test('a failed compile is dropped, and the next caller compiles again', async () => { + const mp4 = writeFixture('failed-compile.mp4', Buffer.concat([ftyp, moov])); + const host = { env: { AGENT_DEVICE_SWIFT_CACHE_DIR: '/cache/failed' }, cwd: directory }; + vi.mocked(compileSwiftSourceText) + .mockClear() + .mockRejectedValueOnce(new AppError('COMMAND_FAILED', 'xcrun timed out after 120000ms')); + + await expect(isPlayableVideo(mp4, host)).rejects.toMatchObject({ code: 'COMMAND_FAILED' }); + await expect(isPlayableVideo(mp4, host)).resolves.toBe(true); + expect(compileSwiftSourceText).toHaveBeenCalledTimes(2); +}); + +test('a budget spent before the validator runs reads as not playable instead of a 1 ms run', async () => { + const mp4 = writeFixture('spent-budget.mp4', Buffer.concat([ftyp, moov])); + const host = { env: { AGENT_DEVICE_SWIFT_CACHE_DIR: '/cache/spent' }, cwd: directory }; + await isPlayableVideo(mp4, host); + vi.mocked(runCmd).mockClear(); + + await expect(isPlayableVideo(mp4, host, Deadline.fromTimeoutMs(0))).resolves.toBe(false); + await expect( + waitForPlayableVideo(mp4, { host, pollMs: 0, deadline: Deadline.fromTimeoutMs(0) }), + ).resolves.toBe(false); + expect(runCmd).not.toHaveBeenCalled(); +}); + +test('the validator run after a compile stays inside the caller deadline', async () => { + const mp4 = writeFixture('run-clamp.mp4', Buffer.concat([ftyp, moov])); + const host = { env: { AGENT_DEVICE_SWIFT_CACHE_DIR: '/cache/run-clamp' }, cwd: directory }; + + await isPlayableVideo(mp4, host, Deadline.fromTimeoutMs(5_000)); + await isPlayableVideo(mp4, host); + + const [withDeadline, withoutDeadline] = vi + .mocked(runCmd) + .mock.calls.map(([, , options]) => options?.timeoutMs); + expect(withDeadline).toBeGreaterThan(0); + expect(withDeadline).toBeLessThanOrEqual(5_000); + expect(withoutDeadline).toBe(10_000); +}); + +test('a validator run that times out reads as not playable, whatever bound stopped it', async () => { + const mp4 = writeFixture('run-cut-short.mp4', Buffer.concat([ftyp, moov])); + const host = { env: { AGENT_DEVICE_SWIFT_CACHE_DIR: '/cache/run-cut-short' }, cwd: directory }; + const timedOut = (timeoutMs: number) => + new AppError('COMMAND_FAILED', `validator timed out after ${timeoutMs}ms`, { timeoutMs }); + vi.mocked(runCmd).mockImplementation(async (_cmd, _args, options) => { + throw timedOut(options!.timeoutMs!); + }); + // A frozen clock keeps the 5 ms budget from passing before the validator runs. + vi.useFakeTimers({ toFake: ['Date'] }); + try { + await expect(isPlayableVideo(mp4, host, Deadline.fromTimeoutMs(5))).resolves.toBe(false); + await expect(isPlayableVideo(mp4, host)).resolves.toBe(false); + expect(vi.mocked(runCmd).mock.calls.map(([, , options]) => options?.timeoutMs)).toEqual([ + 5, 10_000, + ]); + } finally { + vi.useRealTimers(); + vi.mocked(runCmd).mockReset(); + vi.mocked(runCmd).mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); + } +}); + +test('waiting for a playable video checks it once per attempt', async () => { + const mp4 = writeFixture('never-plays.mp4', Buffer.concat([ftyp, moov])); + vi.mocked(runCmd).mockResolvedValue({ exitCode: 1, stdout: '', stderr: '' }); + try { + await expect( + waitForPlayableVideo(mp4, { host: createTestHost(), pollMs: 0, attempts: 2 }), + ).resolves.toBe(false); + expect(runCmd).toHaveBeenCalledTimes(2); + } finally { + vi.mocked(runCmd).mockResolvedValue({ exitCode: 0, stdout: '', stderr: '' }); + } +}); + test('reaches the semantic validator with an MP4 container whose last box runs past the file', async () => { const truncated = Buffer.concat([ftyp, moov]).subarray(0, ftyp.length + moov.length - 4); await expect( diff --git a/packages/capture-kit/src/recording/video.ts b/packages/capture-kit/src/recording/video.ts index ece772e724..df9c38b350 100644 --- a/packages/capture-kit/src/recording/video.ts +++ b/packages/capture-kit/src/recording/video.ts @@ -1,6 +1,6 @@ import fs from 'node:fs'; import { AppError } from '@agent-device/kernel/errors'; -import { runCmd } from '@agent-device/host-kit/command'; +import { isCommandTimeoutError, runCmd } from '@agent-device/host-kit/command'; import { type Deadline, sleep } from '@agent-device/host-kit/retry'; import { buildSwiftToolEnv, compileSwiftSourceText, getSwiftCacheRoot } from './swift-cache.ts'; import type { HostEnvironment } from '@agent-device/kernel/source-value'; @@ -35,8 +35,24 @@ semaphore.wait() exit(exitCode) `.trim(); +// A cold Swift module cache makes the validator compile take over 30 s on a loaded CI runner, so a +// caller with a deadline, such as `record stop`, waits for the compile until that deadline instead. +const VALIDATOR_COMPILE_WAIT_MS = 30_000; +const VALIDATOR_RUN_TIMEOUT_MS = 10_000; + +type SharedValidatorCompile = { + readonly compiled: Promise; + readonly cancel: AbortController; + waiters: number; + settled: boolean; +}; + // A compiled helper is valid for the Swift cache it was built into, so each cache root keeps its own. -const videoValidatorExecutablePaths = new Map>(); +// Callers share one compile, and each waits for it only within its own budget. The last caller to +// stop waiting cancels an unfinished compile: that kills this process's `swiftc`, or stops waiting +// for another process's compile. Nothing owns `swiftc` once its requests end, so a daemon that +// exits would leave it running. +const videoValidatorCompiles = new Map(); export async function waitForStableFile( filePath: string, @@ -69,19 +85,44 @@ export async function waitForStableFile( } } -export async function isPlayableVideo(filePath: string, host: HostEnvironment): Promise { +/** + * Whether the file is a playable video. With a `deadline`, a cold validator compile runs until the + * deadline instead of its own fixed bound. A validator run that times out, or that the deadline + * leaves no time for, reads as not playable, like a poll in `waitForPlayableVideo` that runs out of + * time. A WebM + * verdict reads only the file's structure, with no validator, so the deadline does not apply to it. + */ +export async function isPlayableVideo( + filePath: string, + host: HostEnvironment, + deadline?: Deadline, +): Promise { const container = await readVideoContainerKind(filePath); if (!container) return false; // AVFoundation is the MP4 semantic validator. It does not reliably load WebM on supported // macOS hosts, so WebM completion is established by its EBML document type + Segment marker. if (container === 'webm') return true; try { - const validatorPath = await getVideoValidatorExecutablePath(host); + if (deadline?.isExpired()) return false; + const validatorPath = await waitForValidatorCompile( + host, + deadline?.remainingMs() ?? VALIDATOR_COMPILE_WAIT_MS, + ); + const runTimeoutMs = Math.min( + VALIDATOR_RUN_TIMEOUT_MS, + deadline?.remainingMs() ?? VALIDATOR_RUN_TIMEOUT_MS, + ); + // A budget with no time left reads as not playable; `runCmd` would read a timeout of 0 as none. + if (runTimeoutMs <= 0) return false; const result = await runCmd(validatorPath, [filePath], { allowFailure: true, - timeoutMs: 10_000, + timeoutMs: runTimeoutMs, env: buildSwiftToolEnv(host.env), + }).catch((error: unknown) => { + if (isCommandTimeoutError(error)) return undefined; + throw error; }); + if (!result) return false; if (result.exitCode === 0) { return true; } @@ -94,23 +135,71 @@ export async function isPlayableVideo(filePath: string, host: HostEnvironment): } } -async function getVideoValidatorExecutablePath(host: HostEnvironment): Promise { - const cacheRoot = getSwiftCacheRoot(host.env); - let compiled = videoValidatorExecutablePaths.get(cacheRoot); - if (!compiled) { - compiled = compileSwiftSourceText({ +function joinValidatorCompile(host: HostEnvironment, cacheRoot: string): SharedValidatorCompile { + const existing = videoValidatorCompiles.get(cacheRoot); + if (existing) return existing; + const cancel = new AbortController(); + const shared: SharedValidatorCompile = { + compiled: compileSwiftSourceText({ host, source: VIDEO_VALIDATION_SCRIPT, cacheName: 'video-validator', - timeoutMs: 30_000, - }); - videoValidatorExecutablePaths.set(cacheRoot, compiled); - } + signal: cancel.signal, + }), + cancel, + waiters: 0, + settled: false, + }; + videoValidatorCompiles.set(cacheRoot, shared); + shared.compiled.then( + () => { + shared.settled = true; + }, + () => { + shared.settled = true; + if (videoValidatorCompiles.get(cacheRoot) === shared) + videoValidatorCompiles.delete(cacheRoot); + }, + ); + return shared; +} + +async function waitForValidatorCompile(host: HostEnvironment, waitMs: number): Promise { + const cacheRoot = getSwiftCacheRoot(host.env); + const shared = joinValidatorCompile(host, cacheRoot); + shared.waiters += 1; + let timer: NodeJS.Timeout | undefined; + const expired = new Promise((_, reject) => { + timer = setTimeout( + () => + reject( + new AppError( + 'COMMAND_FAILED', + `The video validator did not compile within ${Math.round(waitMs)}ms`, + { + reason: 'video_validator_compile_timeout', + timeoutMs: waitMs, + retriable: true, + hint: + 'Run record stop again: the Swift modules this compile already built stay ' + + 'cached, so the next compile finishes sooner, and the recording keeps its ' + + 'evidence either way.', + }, + ), + ), + waitMs, + ); + }); try { - return await compiled; - } catch (error) { - videoValidatorExecutablePaths.delete(cacheRoot); - throw error; + return await Promise.race([shared.compiled, expired]); + } finally { + clearTimeout(timer); + shared.waiters -= 1; + if (shared.waiters === 0 && !shared.settled) { + if (videoValidatorCompiles.get(cacheRoot) === shared) + videoValidatorCompiles.delete(cacheRoot); + shared.cancel.abort(); + } } } @@ -127,20 +216,23 @@ function isSwiftVideoValidatorError(error: unknown): boolean { ); } -/** Polls until the file plays, for `attempts` polls or until `deadline` passes, whichever is first. */ +/** + * Polls until the file plays, for `attempts` polls or until `deadline` passes, whichever is first, + * and resolves whether it played. + */ export async function waitForPlayableVideo( filePath: string, options: { host: HostEnvironment; pollMs?: number; attempts?: number; deadline?: Deadline }, -): Promise { +): Promise { const pollMs = options.pollMs ?? 150; const attempts = options.attempts ?? 12; - for (let attempt = 0; attempt < attempts; attempt += 1) { - if (await isPlayableVideo(filePath, options.host)) { - return; + for (let attempt = 1; ; attempt += 1) { + if (await isPlayableVideo(filePath, options.host, options.deadline)) { + return true; } - if (options.deadline?.isExpired()) { - return; + if (attempt >= attempts || options.deadline?.isExpired()) { + return false; } await sleep(pollMs); } diff --git a/src/platform-runtime-screen-recording-finalizer-host.test.ts b/src/platform-runtime-screen-recording-finalizer-host.test.ts index c1728b3d53..9eb0b6f859 100644 --- a/src/platform-runtime-screen-recording-finalizer-host.test.ts +++ b/src/platform-runtime-screen-recording-finalizer-host.test.ts @@ -4,12 +4,14 @@ import { OVERLAY_BUDGET_MS, type overlayRecordingTouches, } from '@agent-device/capture-kit/recording-overlay'; +import { Deadline } from '@agent-device/host-kit/retry'; +import { AppError } from '@agent-device/kernel/errors'; import { createScreenRecordingFinalizer } from './platform-runtime-screen-recording-finalizer-host.ts'; const video = vi.hoisted(() => ({ stable: vi.fn(async () => {}), - playable: vi.fn(async () => {}), - isPlayable: vi.fn(async () => true), + playable: vi.fn(async (_path: string, _options: { deadline?: Deadline }) => true), + isPlayable: vi.fn(async (_path: string, _host: unknown, _deadline?: Deadline) => true), container: vi.fn(async () => true), })); const telemetry = vi.hoisted(() => vi.fn(() => '/tmp/capture.telemetry.json')); @@ -37,7 +39,7 @@ vi.mock('@agent-device/capture-kit/recording-overlay', async (importOriginal) => beforeEach(() => { overlay.mockReset().mockResolvedValue(undefined); video.stable.mockReset().mockResolvedValue(undefined); - video.playable.mockReset().mockResolvedValue(undefined); + video.playable.mockReset().mockResolvedValue(true); video.isPlayable.mockReset().mockResolvedValue(true); video.container.mockReset().mockResolvedValue(true); }); @@ -53,12 +55,13 @@ test('requires stable playable media before publishing finalization telemetry', expect(video.stable).toHaveBeenCalledWith('/tmp/capture.mp4'); expect(video.playable).toHaveBeenCalledWith('/tmp/capture.mp4', { host: expect.objectContaining({ cwd: process.cwd() }), + deadline: expect.any(Deadline), }); expect(result).toEqual({ telemetryPath: '/tmp/capture.telemetry.json' }); }); test('names the retry and the escape for a recording that never became playable', async () => { - video.isPlayable.mockResolvedValueOnce(false); + video.playable.mockResolvedValueOnce(false); await expect( createScreenRecordingFinalizer(createLiveTestHostEnvironment()).complete({ @@ -78,6 +81,24 @@ test('names the retry and the escape for a recording that never became playable' }); }); +test('a validator compile that outlasts the budget reaches the caller as its own retriable error', async () => { + const compileTimeout = new AppError('COMMAND_FAILED', 'The video validator did not compile', { + reason: 'video_validator_compile_timeout', + retriable: true, + }); + video.playable.mockRejectedValueOnce(compileTimeout); + + await expect( + createScreenRecordingFinalizer(createLiveTestHostEnvironment()).complete({ + outputPath: '/tmp/capture.mp4', + showTouches: false, + gestureEvents: [], + targetLabel: 'test recording', + }), + ).rejects.toBe(compileTimeout); + expect(telemetry).not.toHaveBeenCalled(); +}); + test('burns touches into a playable recording without a warning', async () => { const result = await createScreenRecordingFinalizer(createLiveTestHostEnvironment()).complete({ outputPath: '/tmp/capture.mp4', @@ -119,9 +140,36 @@ test('the overlay budget covers the playability checks that run before the overl targetLabel: 'test recording', }); - expect(overlay.mock.calls[0]![0].deadline.remainingMs()).toBe( - OVERLAY_BUDGET_MS - playabilityMs, - ); + const budget = overlay.mock.calls[0]![0].deadline; + expect(budget.remainingMs()).toBe(OVERLAY_BUDGET_MS - playabilityMs); + // The validator compile those checks may run is bounded by the same budget. + expect(video.playable.mock.calls[0]![1].deadline).toBe(budget); + } finally { + vi.useRealTimers(); + } +}); + +test('a recording confirmed playable as the budget runs out is not refused by a second check', async () => { + // A check after the budget is spent reads as not playable, so the finalizer decides on the + // verdict of the wait that confirmed the file rather than asking again. Any second check, with + // the spent budget or without one, refuses here. + vi.useFakeTimers({ toFake: ['Date'] }); + try { + video.isPlayable.mockResolvedValue(false); + video.playable.mockImplementationOnce(async () => { + vi.setSystemTime(Date.now() + OVERLAY_BUDGET_MS); + return true; + }); + + await expect( + createScreenRecordingFinalizer(createLiveTestHostEnvironment()).complete({ + outputPath: '/tmp/capture.mp4', + showTouches: false, + gestureEvents: [], + targetLabel: 'test recording', + }), + ).resolves.toEqual({ telemetryPath: '/tmp/capture.telemetry.json' }); + expect(video.isPlayable).not.toHaveBeenCalled(); } finally { vi.useRealTimers(); } diff --git a/src/platform-runtime-screen-recording-finalizer-host.ts b/src/platform-runtime-screen-recording-finalizer-host.ts index dee28a5b67..d6af20ae47 100644 --- a/src/platform-runtime-screen-recording-finalizer-host.ts +++ b/src/platform-runtime-screen-recording-finalizer-host.ts @@ -33,15 +33,16 @@ async function finalizeScreenRecording( host: HostEnvironment, input: Parameters[0], ) { - const overlayDeadline = startRecordingOverlayBudget(); + const finalizeDeadline = startRecordingOverlayBudget(); await waitForStableFile(input.outputPath); - await waitForPlayableVideo(input.outputPath, { host }); - if (!(await isPlayableVideo(input.outputPath, host))) throw unplayableRecording(input.outputPath); + if (!(await waitForPlayableVideo(input.outputPath, { host, deadline: finalizeDeadline }))) { + throw unplayableRecording(input.outputPath); + } const telemetryPath = persistRecordingTelemetry({ recording: { outPath: input.outputPath, gestureEvents: [...input.gestureEvents] }, }); if (!input.showTouches || input.gestureEvents.length === 0) return { telemetryPath }; - return await overlayTouches(host, input, telemetryPath, overlayDeadline); + return await overlayTouches(host, input, telemetryPath, finalizeDeadline); } async function overlayTouches(