Skip to content

Commit b08d6b9

Browse files
thymikeeclaude
thymikee
and
claude
committed
fix(recording): name the overlay budget only when the export itself ran out of it
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
1 parent 2ac5308 commit b08d6b9

5 files changed

Lines changed: 91 additions & 20 deletions

File tree

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

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,45 @@ test('overlay leaves the video as recorded when compiling the helper spent its b
183183

184184
await expect(overlayRecordingTouches({ videoPath, telemetryPath })).rejects.toMatchObject({
185185
code: 'COMMAND_FAILED',
186-
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`,
186+
message: 'Failed to add touch overlays to the recording',
187187
});
188188
expect(mockRunCmd.mock.calls.filter(([cmd]) => cmd !== 'xcrun')).toHaveLength(0);
189189
expect(fs.readFileSync(videoPath, 'utf8')).toBe('original');
190190
});
191+
192+
test('overlay names its budget when the export ran out of it', async () => {
193+
vi.useFakeTimers({ toFake: ['Date'] });
194+
const videoPath = path.join(tmpDir, 'recording.mp4');
195+
const telemetryPath = path.join(tmpDir, 'recording.gesture-telemetry.json');
196+
fs.writeFileSync(videoPath, 'original');
197+
fs.writeFileSync(telemetryPath, '{"events":[]}');
198+
199+
await overlayRecordingTouches({ videoPath, telemetryPath });
200+
fs.writeFileSync(videoPath, 'original');
201+
mockRunCmd.mockImplementationOnce(async () => {
202+
vi.setSystemTime(Date.now() + OVERLAY_BUDGET_MS);
203+
throw new AppError('COMMAND_FAILED', 'recording-overlay exited with code 1');
204+
});
205+
206+
await expect(overlayRecordingTouches({ videoPath, telemetryPath })).rejects.toMatchObject({
207+
code: 'COMMAND_FAILED',
208+
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`,
209+
});
210+
expect(fs.readFileSync(videoPath, 'utf8')).toBe('original');
211+
});
212+
213+
test('overlay keeps the plain failure when the export failed with budget to spare', async () => {
214+
const videoPath = path.join(tmpDir, 'recording.mp4');
215+
const telemetryPath = path.join(tmpDir, 'recording.gesture-telemetry.json');
216+
fs.writeFileSync(videoPath, 'original');
217+
fs.writeFileSync(telemetryPath, '{"events":[]}');
218+
219+
await overlayRecordingTouches({ videoPath, telemetryPath });
220+
mockRunCmd.mockImplementationOnce(async () => {
221+
throw new AppError('COMMAND_FAILED', 'recording-overlay exited with code 1');
222+
});
223+
224+
await expect(overlayRecordingTouches({ videoPath, telemetryPath })).rejects.toMatchObject({
225+
message: 'Failed to add touch overlays to the recording',
226+
});
227+
});

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

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ async function exportProcessedVideo(params: {
6767
await waitForPlayableVideo(videoPath);
6868

6969
const outputPath = temporarySiblingVideoPath(videoPath);
70+
let exportOverran = false;
7071
try {
7172
const executablePath = await compileSwiftSourceFile({
7273
sourcePath: scriptPath,
@@ -82,26 +83,30 @@ async function exportProcessedVideo(params: {
8283
`No time was left for the export within the ${params.budgetMs}ms budget`,
8384
);
8485
}
85-
await runCmd(
86-
executablePath,
87-
[
88-
'--input',
89-
videoPath,
90-
'--output',
91-
outputPath,
92-
...scriptArgs,
93-
'--timeout-ms',
94-
String(Math.round(exportMs)),
95-
],
96-
{ timeoutMs: helperMs, env: buildSwiftToolEnv() },
97-
);
86+
try {
87+
await runCmd(
88+
executablePath,
89+
[
90+
'--input',
91+
videoPath,
92+
'--output',
93+
outputPath,
94+
...scriptArgs,
95+
'--timeout-ms',
96+
String(Math.round(exportMs)),
97+
],
98+
{ timeoutMs: helperMs, env: buildSwiftToolEnv() },
99+
);
100+
} catch (error) {
101+
exportOverran = deadline.remainingMs() <= HELPER_EXIT_GRACE_MS;
102+
throw error;
103+
}
98104
await waitForPlayableVideo(outputPath);
99105
fs.renameSync(outputPath, videoPath);
100106
} catch (error) {
101-
const description =
102-
deadline.remainingMs() <= HELPER_EXIT_GRACE_MS
103-
? `${commandDescription}: the export did not finish within its ${params.budgetMs}ms budget; a lower --fps renders faster`
104-
: commandDescription;
107+
const description = exportOverran
108+
? `${commandDescription}: the export did not finish within its ${params.budgetMs}ms budget; a lower --fps renders faster`
109+
: commandDescription;
105110
const cause =
106111
error instanceof AppError
107112
? error

‎packages/contracts/src/screen-recording-runtime.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,10 @@ export type ScreenRecordingLiveSnapshot = Readonly<{
7272
recordOnlySession: boolean;
7373
activeSessionApp?: RecordingAppIdentity;
7474
exportQuality?: RecordingExportQuality;
75-
/** The caller's `--fps`: the capture rate where the recorder takes one, and the overlay's cap. */
75+
/**
76+
* The caller's `--fps`: the capture rate where the recorder takes one. The touch overlay renders
77+
* at most this many frames a second, and never more than 30.
78+
*/
7679
fps?: number;
7780
gestureEvents: readonly RecordingGestureEvent[];
7881
touchReferenceFrame?: GestureReferenceFrame;

‎packages/platform-apple/src/recording/completion.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,29 @@ test('hands the caller fps to the finalizer that burns the touch overlay', async
5454

5555
expect(requests).toEqual([expect.objectContaining({ showTouches: true, fps: 15 })]);
5656
});
57+
58+
test('hands the finalizer no fps when the caller asked for none', async () => {
59+
const requests: Readonly<{ fps?: number }>[] = [];
60+
await completeAppleRecording({
61+
host: appleRecordingHost({
62+
complete: async (input) => {
63+
requests.push(input);
64+
return {};
65+
},
66+
}),
67+
snapshot: {
68+
backend: 'simctl',
69+
outPath: '/tmp/capture.mp4',
70+
startedAt: 1,
71+
scope: 'app',
72+
showTouches: true,
73+
recordOnlySession: false,
74+
gestureEvents: [],
75+
},
76+
targetLabel: 'iOS recording',
77+
stopObservation: { recorder: 'confirmed' },
78+
});
79+
80+
expect(requests).toHaveLength(1);
81+
expect(requests[0]).not.toHaveProperty('fps');
82+
});

‎website/docs/docs/commands.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,7 @@ agent-device record stop # Stop active recording
11101110
- 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.
11111111
- Android `screenrecord` encodes a frame only when the screen changes, so a clip ends at the last frame the recorder encoded instead of at `record stop`: a window that ends on an unchanged screen yields a shorter video, while every on-screen change inside the window stays at its real offset in it. `record stop` reports `durationMs` as host wall clock from `record start` until the export finished, and when the video can be measured it also reports `capturedDurationMs` and warns with how much of the window that video covers.
11121112
- Limrun iOS and Android direct sessions record the whole simulator or emulator screen through the provider's server-side recorder, so every `--scope` captures the same frame and `--fps` and `--hide-touches` are refused with `INVALID_ARGS` before any device work. `record stop` asks the instance to stop once and then downloads the served MP4 to the output path; that download is bounded to end inside the request window, so a slow or dropped transfer ends typed, leaves no file behind, and is retried by the next `record stop` from the same URL while the instance lives. Nothing survives a daemon restart: the recording is `unreattachable` and the instance disposes the file when the lease is released.
1113-
- `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 local or 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`.
1113+
- `record stop` is safe to repeat. When its request window ends while the daemon is still exporting, on a local or 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`.
11141114
- A recording answers two independent questions, and `record stop` reports both: whether a playable export exists, and whether the recorder stopped. `recorder` is `confirmed` when the recorder exited or acknowledged a stop meant for this recording, and `lost` when the session holding it died — an Apple recording invalidated by a runner restart. `nativePathDisposition` says what became of the artifact path the recorder itself writes to: `retirable` while that file still sits there owed a removal, and `retired` once its removal was verified. Both are optional disclosures — the export is served either way, a replay of a recording stopped before they existed omits them, and so does a backend whose recorder writes the served file itself. The vocabulary is deliberately wider than today's answers: `recorder: unconfirmed` (a probe that could not be read, or no exit inside the stop budget), the identity-mismatch reasons under `lost`, and `nativePathDisposition: pending` are declared by [ADR 0024](https://github.com/callstack/agent-device/blob/main/docs/adr/0024-screen-recording-provable-signal.md) for the steps that gain those probes, and no stop reports them yet.
11151115
11161116
- `record contact-sheet <video.mp4> [--out <sheet.png>]` turns a recording you already exported into one PNG: the frames where the screen visibly changed, laid out in a grid and each labeled with its elapsed time (`HH:MM:SS.mmm`) on the clip timeline. It is how an agent reads a recording it cannot play. It reads the file you pass — no session, no device, no daemon — so it can rebuild an old take and the sheet can only describe screens that export contains. Any backend that exports MP4 works (Apple, Android, HarmonyOS, Limrun); a WebM recording from the web backend is refused with `details.reason: contact_sheet_container_unsupported`. The default output is `<recording>.contact-sheet.png`, and `--out` is refused when it points back at the recording itself, because a sheet is derived from a take and never replaces it. `--json` reports each cell's time and changed-pixel share alongside `sampledFrameCount`, `decodedFrameCount`, and `skippedSampleCount`.

0 commit comments

Comments
 (0)