Skip to content

Commit 2d09bbb

Browse files
thymikeethymikee
committed
fix(record): start the touch overlay budget when record stop starts finalizing
The overlay budget started when the overlay did, after the playability checks that run first. On a cold host those compile their validator (measured 5.8 s, 21.6 s at background QoS; its own budget is 30 s), so the overlay could still spend its full 70 s on top and push record stop past the 90 s request envelope: the client exits 1 with "Daemon request timed out" while the daemon finishes with an overlayWarning that the next record stop replays. The finalizer now starts the budget before those checks and hands it to the overlay, so their time comes out of the overlay's share instead of being added to it. The overlay no longer repeats the stable-file and playability waits the finalizer has just run on the same file.
1 parent 2a2f3f2 commit 2d09bbb

6 files changed

Lines changed: 139 additions & 34 deletions

File tree

‎packages/capture-kit/src/recording/__tests__/overlay.test.ts‎

Lines changed: 80 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,14 @@ vi.mock('@agent-device/host-kit/command', async (importOriginal) => {
2424
});
2525

2626
vi.mock('../video.ts', () => ({
27-
waitForStableFile: vi.fn(async () => {}),
2827
waitForPlayableVideo: vi.fn(async () => {}),
2928
}));
3029

31-
import { OVERLAY_BUDGET_MS, overlayRecordingTouches } from '../overlay.ts';
30+
import {
31+
OVERLAY_BUDGET_MS,
32+
overlayRecordingTouches,
33+
startRecordingOverlayBudget,
34+
} from '../overlay.ts';
3235
import { AppError } from '@agent-device/kernel/errors';
3336
import { runCmd } from '@agent-device/host-kit/command';
3437

@@ -58,9 +61,17 @@ test('overlay burns touches through a cached helper and same-directory temp outp
5861
fs.writeFileSync(videoPath, 'original');
5962
fs.writeFileSync(telemetryPath, '{"events":[]}');
6063

61-
await overlayRecordingTouches({ videoPath, telemetryPath });
64+
await overlayRecordingTouches({
65+
videoPath,
66+
telemetryPath,
67+
deadline: startRecordingOverlayBudget(),
68+
});
6269
fs.writeFileSync(videoPath, 'second original');
63-
await overlayRecordingTouches({ videoPath, telemetryPath });
70+
await overlayRecordingTouches({
71+
videoPath,
72+
telemetryPath,
73+
deadline: startRecordingOverlayBudget(),
74+
});
6475

6576
const compileCalls = mockRunCmd.mock.calls.filter(([cmd, args]) => {
6677
return cmd === 'xcrun' && args[0] === 'swiftc';
@@ -96,7 +107,9 @@ test('overlay preserves Swift helper compile hints', async () => {
96107
throw new AppError('COMMAND_FAILED', 'Timed out waiting for Swift cache lock', { hint });
97108
});
98109

99-
await expect(overlayRecordingTouches({ videoPath, telemetryPath })).rejects.toMatchObject({
110+
await expect(
111+
overlayRecordingTouches({ videoPath, telemetryPath, deadline: startRecordingOverlayBudget() }),
112+
).rejects.toMatchObject({
100113
code: 'COMMAND_FAILED',
101114
message: 'Failed to add touch overlays to the recording',
102115
details: {
@@ -111,7 +124,11 @@ test('overlay defaults to the fast medium export preset', async () => {
111124
fs.writeFileSync(videoPath, 'original');
112125
fs.writeFileSync(telemetryPath, '{"events":[]}');
113126

114-
await overlayRecordingTouches({ videoPath, telemetryPath });
127+
await overlayRecordingTouches({
128+
videoPath,
129+
telemetryPath,
130+
deadline: startRecordingOverlayBudget(),
131+
});
115132

116133
expect(helperScriptArgs()).toEqual(
117134
expect.arrayContaining(['--events', telemetryPath, '--quality', 'medium']),
@@ -124,7 +141,12 @@ test('overlay forwards the requested high export preset', async () => {
124141
fs.writeFileSync(videoPath, 'original');
125142
fs.writeFileSync(telemetryPath, '{"events":[]}');
126143

127-
await overlayRecordingTouches({ videoPath, telemetryPath, exportQuality: 'high' });
144+
await overlayRecordingTouches({
145+
videoPath,
146+
telemetryPath,
147+
deadline: startRecordingOverlayBudget(),
148+
exportQuality: 'high',
149+
});
128150

129151
expect(helperScriptArgs()).toEqual(
130152
expect.arrayContaining(['--events', telemetryPath, '--quality', 'high']),
@@ -137,11 +159,20 @@ test('overlay caps the helper frame rate at the caller fps only when one was ask
137159
fs.writeFileSync(videoPath, 'original');
138160
fs.writeFileSync(telemetryPath, '{"events":[]}');
139161

140-
await overlayRecordingTouches({ videoPath, telemetryPath });
162+
await overlayRecordingTouches({
163+
videoPath,
164+
telemetryPath,
165+
deadline: startRecordingOverlayBudget(),
166+
});
141167
expect(helperScriptArgs()).not.toContain('--max-fps');
142168

143169
mockRunCmd.mockClear();
144-
await overlayRecordingTouches({ videoPath, telemetryPath, fps: 15 });
170+
await overlayRecordingTouches({
171+
videoPath,
172+
telemetryPath,
173+
deadline: startRecordingOverlayBudget(),
174+
fps: 15,
175+
});
145176
const args = helperScriptArgs();
146177
expect(args[args.indexOf('--max-fps') + 1]).toBe('15');
147178
});
@@ -152,7 +183,11 @@ test('overlay hands the helper what is left of its budget, inside the record req
152183
fs.writeFileSync(videoPath, 'original');
153184
fs.writeFileSync(telemetryPath, '{"events":[]}');
154185

155-
await overlayRecordingTouches({ videoPath, telemetryPath });
186+
await overlayRecordingTouches({
187+
videoPath,
188+
telemetryPath,
189+
deadline: startRecordingOverlayBudget(),
190+
});
156191

157192
const compileCall = mockRunCmd.mock.calls.find(([cmd]) => cmd === 'xcrun');
158193
const helperCall = mockRunCmd.mock.calls.find(([cmd]) => cmd !== 'xcrun');
@@ -166,6 +201,22 @@ test('overlay hands the helper what is left of its budget, inside the record req
166201
expect(exportMs).toBeLessThan(helperCall?.[2]?.timeoutMs ?? 0);
167202
});
168203

204+
test('overlay hands the helper only what the finalization left of the budget', async () => {
205+
vi.useFakeTimers({ toFake: ['Date'] });
206+
const videoPath = path.join(tmpDir, 'recording.mp4');
207+
const telemetryPath = path.join(tmpDir, 'recording.gesture-telemetry.json');
208+
fs.writeFileSync(videoPath, 'original');
209+
fs.writeFileSync(telemetryPath, '{"events":[]}');
210+
const spentMs = 25_000;
211+
const deadline = startRecordingOverlayBudget();
212+
vi.setSystemTime(Date.now() + spentMs);
213+
214+
await overlayRecordingTouches({ videoPath, telemetryPath, deadline });
215+
216+
const helperCall = mockRunCmd.mock.calls.find(([cmd]) => cmd !== 'xcrun');
217+
expect(helperCall?.[2]?.timeoutMs).toBe(OVERLAY_BUDGET_MS - spentMs);
218+
});
219+
169220
test('overlay leaves the video as recorded when compiling the helper spent its budget', async () => {
170221
vi.useFakeTimers({ toFake: ['Date'] });
171222
const videoPath = path.join(tmpDir, 'recording.mp4');
@@ -181,7 +232,9 @@ test('overlay leaves the video as recorded when compiling the helper spent its b
181232
return { stdout: '', stderr: '', exitCode: 0 };
182233
});
183234

184-
await expect(overlayRecordingTouches({ videoPath, telemetryPath })).rejects.toMatchObject({
235+
await expect(
236+
overlayRecordingTouches({ videoPath, telemetryPath, deadline: startRecordingOverlayBudget() }),
237+
).rejects.toMatchObject({
185238
code: 'COMMAND_FAILED',
186239
message: 'Failed to add touch overlays to the recording',
187240
});
@@ -196,14 +249,20 @@ test('overlay names its budget when the export ran out of it', async () => {
196249
fs.writeFileSync(videoPath, 'original');
197250
fs.writeFileSync(telemetryPath, '{"events":[]}');
198251

199-
await overlayRecordingTouches({ videoPath, telemetryPath });
252+
await overlayRecordingTouches({
253+
videoPath,
254+
telemetryPath,
255+
deadline: startRecordingOverlayBudget(),
256+
});
200257
fs.writeFileSync(videoPath, 'original');
201258
mockRunCmd.mockImplementationOnce(async () => {
202259
vi.setSystemTime(Date.now() + OVERLAY_BUDGET_MS);
203260
throw new AppError('COMMAND_FAILED', 'recording-overlay exited with code 1');
204261
});
205262

206-
await expect(overlayRecordingTouches({ videoPath, telemetryPath })).rejects.toMatchObject({
263+
await expect(
264+
overlayRecordingTouches({ videoPath, telemetryPath, deadline: startRecordingOverlayBudget() }),
265+
).rejects.toMatchObject({
207266
code: 'COMMAND_FAILED',
208267
message: `Failed to add touch overlays to the recording: the export did not finish within its ${OVERLAY_BUDGET_MS}ms budget; a lower --fps renders faster`,
209268
});
@@ -216,12 +275,18 @@ test('overlay keeps the plain failure when the export failed with budget to spar
216275
fs.writeFileSync(videoPath, 'original');
217276
fs.writeFileSync(telemetryPath, '{"events":[]}');
218277

219-
await overlayRecordingTouches({ videoPath, telemetryPath });
278+
await overlayRecordingTouches({
279+
videoPath,
280+
telemetryPath,
281+
deadline: startRecordingOverlayBudget(),
282+
});
220283
mockRunCmd.mockImplementationOnce(async () => {
221284
throw new AppError('COMMAND_FAILED', 'recording-overlay exited with code 1');
222285
});
223286

224-
await expect(overlayRecordingTouches({ videoPath, telemetryPath })).rejects.toMatchObject({
287+
await expect(
288+
overlayRecordingTouches({ videoPath, telemetryPath, deadline: startRecordingOverlayBudget() }),
289+
).rejects.toMatchObject({
225290
message: 'Failed to add touch overlays to the recording',
226291
});
227292
});

‎packages/capture-kit/src/recording/overlay.ts‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
compileSwiftSourceFile,
1010
resolveRecordingScriptPath,
1111
} from './swift-cache.ts';
12-
import { waitForPlayableVideo, waitForStableFile } from './video.ts';
12+
import { waitForPlayableVideo } from './video.ts';
1313
import {
1414
DEFAULT_RECORDING_EXPORT_QUALITY,
1515
type RecordingExportQuality,
@@ -27,11 +27,18 @@ export function getRecordingOverlaySupportWarning(
2727
/**
2828
* `record stop` is one daemon request, which the client abandons after `record`'s 90 s envelope:
2929
* the caller gets "Daemon request timed out" while the daemon finishes the stop. So the overlay must
30-
* end well inside it. Compiling the helper, its export and its exit all come out of this budget;
31-
* past it the stop keeps the raw video and its gesture telemetry, with a warning.
30+
* end well inside it. The budget starts with the stop's finalization, so the playability checks
31+
* before the overlay (on a cold host they compile their validator first) come out of it, as do
32+
* compiling the helper, its export and its exit; past it the stop keeps the raw video and its
33+
* gesture telemetry, with a warning.
3234
*/
3335
export const OVERLAY_BUDGET_MS = 70_000;
3436

37+
/** Starts the overlay budget. Call it when the stop's finalization starts, not when the overlay does. */
38+
export function startRecordingOverlayBudget(): Deadline {
39+
return Deadline.fromTimeoutMs(OVERLAY_BUDGET_MS);
40+
}
41+
3542
/**
3643
* The helper's time outside its export: starting up, then verifying the composited output or
3744
* cancelling the export, and exiting.
@@ -59,13 +66,9 @@ async function exportProcessedVideo(params: {
5966
scriptPath: string;
6067
scriptArgs: string[];
6168
commandDescription: string;
62-
budgetMs: number;
69+
deadline: Deadline;
6370
}): Promise<void> {
64-
const { videoPath, scriptPath, scriptArgs, commandDescription } = params;
65-
const deadline = Deadline.fromTimeoutMs(params.budgetMs);
66-
await waitForStableFile(videoPath);
67-
await waitForPlayableVideo(videoPath);
68-
71+
const { videoPath, scriptPath, scriptArgs, commandDescription, deadline } = params;
6972
const outputPath = temporarySiblingVideoPath(videoPath);
7073
let exportOverran = false;
7174
try {
@@ -80,7 +83,7 @@ async function exportProcessedVideo(params: {
8083
if (exportMs <= 0) {
8184
throw new AppError(
8285
'COMMAND_FAILED',
83-
`No time was left for the export within the ${params.budgetMs}ms budget`,
86+
`No time was left for the export within the ${OVERLAY_BUDGET_MS}ms budget`,
8487
);
8588
}
8689
try {
@@ -105,7 +108,7 @@ async function exportProcessedVideo(params: {
105108
fs.renameSync(outputPath, videoPath);
106109
} catch (error) {
107110
const description = exportOverran
108-
? `${commandDescription}: the export did not finish within its ${params.budgetMs}ms budget; a lower --fps renders faster`
111+
? `${commandDescription}: the export did not finish within its ${OVERLAY_BUDGET_MS}ms budget; a lower --fps renders faster`
109112
: commandDescription;
110113
const cause =
111114
error instanceof AppError
@@ -144,6 +147,8 @@ export async function overlayRecordingTouches(params: {
144147
/** The caller's `--fps`; the helper renders at most this many frames a second, capped at 30. */
145148
fps?: number;
146149
targetLabel?: string;
150+
/** From `startRecordingOverlayBudget`, taken when the stop's finalization started. */
151+
deadline: Deadline;
147152
}): Promise<void> {
148153
const {
149154
videoPath,
@@ -162,6 +167,6 @@ export async function overlayRecordingTouches(params: {
162167
...(params.fps === undefined ? [] : ['--max-fps', String(params.fps)]),
163168
],
164169
commandDescription: `Failed to add touch overlays to the ${targetLabel}`,
165-
budgetMs: OVERLAY_BUDGET_MS,
170+
deadline: params.deadline,
166171
});
167172
}

‎src/__tests__/command-descriptor-timeout-policy.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -332,9 +332,9 @@ test('snapshot uses the standard daemon request timeout with an explicit overrid
332332
});
333333

334334
test('the touch overlay budget leaves record stop room inside its request envelope', () => {
335-
// The recorder stop, the copies and the playability checks around the overlay are outside its
336-
// budget (about 1 s for a 10-minute simulator recording), so the envelope keeps a reserve for
337-
// them beyond it.
335+
// The recorder stop and the copies before finalization, and the playability check after the
336+
// overlay, are outside its budget (about 1 s for a 10-minute simulator recording), so the envelope
337+
// keeps a reserve for them beyond it.
338338
const reserveMs = 20_000;
339339
const envelopeMs = resolveCommandRequestTimeoutMs(resolveCommandTimeoutPolicy('record'), {
340340
positionals: ['stop'],

‎src/platform-runtime-screen-recording-finalizer-host.test.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
import { beforeEach, expect, test, vi } from 'vitest';
2+
import {
3+
OVERLAY_BUDGET_MS,
4+
type overlayRecordingTouches,
5+
} from '@agent-device/capture-kit/recording-overlay';
26
import { createScreenRecordingFinalizer } from './platform-runtime-screen-recording-finalizer-host.ts';
37

48
const video = vi.hoisted(() => ({
@@ -8,7 +12,7 @@ const video = vi.hoisted(() => ({
812
container: vi.fn(async () => true),
913
}));
1014
const telemetry = vi.hoisted(() => vi.fn(() => '/tmp/capture.telemetry.json'));
11-
const overlay = vi.hoisted(() => vi.fn(async () => {}));
15+
const overlay = vi.hoisted(() => vi.fn<typeof overlayRecordingTouches>(async () => {}));
1216
vi.mock('@agent-device/capture-kit/recording-video', () => ({
1317
waitForStableFile: video.stable,
1418
waitForPlayableVideo: video.playable,
@@ -18,7 +22,8 @@ vi.mock('@agent-device/capture-kit/recording-video', () => ({
1822
vi.mock('@agent-device/capture-kit/recording-telemetry', () => ({
1923
persistRecordingTelemetry: telemetry,
2024
}));
21-
vi.mock('@agent-device/capture-kit/recording-overlay', () => ({
25+
vi.mock('@agent-device/capture-kit/recording-overlay', async (importOriginal) => ({
26+
...(await importOriginal<typeof import('@agent-device/capture-kit/recording-overlay')>()),
2227
getRecordingOverlaySupportWarning: () => undefined,
2328
overlayRecordingTouches: overlay,
2429
}));
@@ -92,6 +97,31 @@ test('hands the caller fps to the touch overlay', async () => {
9297
expect(overlay).toHaveBeenCalledWith(expect.objectContaining({ fps: 15 }));
9398
});
9499

100+
test('the overlay budget covers the playability checks that run before the overlay', async () => {
101+
// A cold host compiles the playability validator before the overlay starts; that time comes out
102+
// of the overlay budget rather than being added to it.
103+
vi.useFakeTimers({ toFake: ['Date'] });
104+
try {
105+
const playabilityMs = 25_000;
106+
video.stable.mockImplementationOnce(async () => {
107+
vi.setSystemTime(Date.now() + playabilityMs);
108+
});
109+
110+
await createScreenRecordingFinalizer().complete({
111+
outputPath: '/tmp/capture.mp4',
112+
showTouches: true,
113+
gestureEvents: [{ kind: 'tap', tMs: 500, x: 10, y: 20 }],
114+
targetLabel: 'test recording',
115+
});
116+
117+
expect(overlay.mock.calls[0]![0].deadline.remainingMs()).toBe(
118+
OVERLAY_BUDGET_MS - playabilityMs,
119+
);
120+
} finally {
121+
vi.useRealTimers();
122+
}
123+
});
124+
95125
test('#2707 reports a dropped overlay and keeps the raw when compositing throws', async () => {
96126
// overlay.ts verifies the composite and only renames it over the raw on success, so a compositor
97127
// that fails its own #2707 checks throws before any rename: the raw file survives and the caller

‎src/platform-runtime-screen-recording-finalizer-host.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import { AppError } from '@agent-device/kernel/errors';
22
import { RECORDING_OUTPUT_UNPLAYABLE_REASON } from '@agent-device/contracts/screen-recording-runtime';
33
import type { ScreenRecordingRuntimeHost } from '@agent-device/contracts/screen-recording-runtime-host';
4+
import type { Deadline } from '@agent-device/host-kit/retry';
45
import {
56
getRecordingOverlaySupportWarning,
67
overlayRecordingTouches,
8+
startRecordingOverlayBudget,
79
} from '@agent-device/capture-kit/recording-overlay';
810
import { persistRecordingTelemetry } from '@agent-device/capture-kit/recording-telemetry';
911
import {
@@ -24,19 +26,21 @@ async function sniffRecording(input: Readonly<{ outputPath: string }>): Promise<
2426
async function finalizeScreenRecording(
2527
input: Parameters<ScreenRecordingRuntimeHost['finalize']['complete']>[0],
2628
) {
29+
const overlayDeadline = startRecordingOverlayBudget();
2730
await waitForStableFile(input.outputPath);
2831
await waitForPlayableVideo(input.outputPath);
2932
if (!(await isPlayableVideo(input.outputPath))) throw unplayableRecording(input.outputPath);
3033
const telemetryPath = persistRecordingTelemetry({
3134
recording: { outPath: input.outputPath, gestureEvents: [...input.gestureEvents] },
3235
});
3336
if (!input.showTouches || input.gestureEvents.length === 0) return { telemetryPath };
34-
return await overlayTouches(input, telemetryPath);
37+
return await overlayTouches(input, telemetryPath, overlayDeadline);
3538
}
3639

3740
async function overlayTouches(
3841
input: Parameters<ScreenRecordingRuntimeHost['finalize']['complete']>[0],
3942
telemetryPath: string,
43+
deadline: Deadline,
4044
) {
4145
const warning = getRecordingOverlaySupportWarning();
4246
if (warning) return { telemetryPath, overlayWarning: warning };
@@ -47,6 +51,7 @@ async function overlayTouches(
4751
exportQuality: input.exportQuality,
4852
...(input.fps === undefined ? {} : { fps: input.fps }),
4953
targetLabel: input.targetLabel,
54+
deadline,
5055
});
5156
if (!(await isPlayableVideo(input.outputPath))) {
5257
throw new Error('recording post-processing produced an unplayable video');

‎website/docs/docs/commands.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -945,7 +945,7 @@ agent-device record stop # Stop active recording
945945
- `record start` defaults to app scope and needs a session from `open <app>`. Use `--scope device` or `--scope system` for whole-screen capture (multiple apps, settings, home screen) where the backend supports it.
946946
- iOS recording works on simulators and CoreDevice-backed physical devices. Physical devices capture runner screenshots, not a native video stream: no audio, dropped frames are expected, and the default is 15 FPS. `--fps <n>` (1-120) caps the rate; a busy app or long command yields fewer frames.
947947
- `--quality medium|high` (default `medium`) sets export quality on Android and Limrun; Apple export always keeps the captured resolution. Numeric `5`-`7` map to `medium` and `8`-`10` to `high`.
948-
- With touch visualization on, recordings also produce a gesture telemetry sidecar. Burned-in touch overlays export only on macOS hosts; elsewhere `record stop` returns the raw video and telemetry with `overlayWarning`. The overlay renders at most 30 FPS (or `--fps` if lower) within a fixed budget; an export past the budget returns the raw video with `overlayWarning`.
948+
- With touch visualization on, recordings also produce a gesture telemetry sidecar. Burned-in touch overlays export only on macOS hosts; elsewhere `record stop` returns the raw video and telemetry with `overlayWarning`. The overlay renders at most 30 FPS (or `--fps` if lower) within a fixed budget, which also covers the video checks that run before it; an export past the budget returns the raw video with `overlayWarning`. On a host that has not run a recording before, part of that budget goes to one-time tool setup, so the first `record stop` of a long recording is more likely to return the raw video.
949949
- Android uses `adb shell screenrecord` (180s limit per clip). Longer recordings are split into MP4 chunks while the daemon stays alive; after a daemon restart, `record stop` recovers only chunks listed in the device manifest. A clip can end before `record stop` when the screen stopped changing; `durationMs` is wall-clock time, and `capturedDurationMs` plus a warning report how much the video covers.
950950
- Limrun sessions always record the whole screen and refuse `--fps` and `--hide-touches` with `INVALID_ARGS`. A failed download is retried by the next `record stop`; nothing survives a daemon restart.
951951
- A busy iOS simulator recording slot returns non-retriable `DEVICE_IN_USE` with `details.reason: apple_simulator_recording_busy`. Run `record stop` in the session that owns the recording; if no recording is active, ask the host operator to restart the CoreSimulator stream service.

0 commit comments

Comments
 (0)