Skip to content

Commit 6903130

Browse files
thymikeethymikee
authored
fix(ios): serve a simulator recording whose recorder died with its daemon (#2736)
* fix(ios): serve a simulator recording whose recorder died with its daemon A simctl reattach answered `missing` as soon as the recorder process was gone, which is the ordinary state after daemon loss, so a retried `record stop` threw `resource-missing` forever even with the recorder's video on disk. The simctl descriptor now carries the caller-facing export coordinates, so a proven-gone recorder whose file still passes the container sniff reattaches as a handle that runs the same stop-and-export sequence the live recording would have run, and discloses the touch overlay whose events did not survive the daemon. A manifest written without those coordinates is answered exactly as it was before, because it cannot name an export it never recorded. * fix(ios): resume a recovered simulator export from what the first stop journaled A recorder that died with its daemon proved nothing about the export its file can still become, and neither did a manifest whose stop had already collected a copy or journaled a finalization. Reattach decided from the recorder's own path alone, so a retry after those steps found no file there and reported a loss the manifest contradicted. It now asks what a resumed stop would still have to read, which the shared stop sequence answers from the checkpoints the first attempt wrote. The coordinates a recovered export needs are the caller-facing keys of the live snapshot, so the facet is a Pick of it, encoded and restored by spread, and validated by the recording-facts validator Android's descriptor already needed — moved to capture-kit so both backends answer with the same strictness. A recovered handle refuses an overlay whose gesture events died with the daemon instead of running the overlay pass with nothing to burn in and then reporting it unavailable. * chore(gates): declare the capture-kit recording-facts subpath in the boundary enumeration
1 parent a798792 commit 6903130

11 files changed

Lines changed: 724 additions & 94 deletions

File tree

‎packages/capture-kit/package.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,10 @@
166166
"types": "./src/recording/artifact.fixtures.ts",
167167
"default": "./src/recording/artifact.fixtures.ts"
168168
},
169+
"./recording-facts": {
170+
"types": "./src/recording/recording-facts.ts",
171+
"default": "./src/recording/recording-facts.ts"
172+
},
169173
"./recording-mp4-fixtures": {
170174
"types": "./src/recording/mp4.fixtures.ts",
171175
"default": "./src/recording/mp4.fixtures.ts"

‎packages/capture-kit/src/recording/recording-facts.test.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import assert from 'node:assert/strict';
2+
import { test } from 'vitest';
3+
import { RECORDING_FACTS_KEYS, recordingFactsAreValid } from './recording-facts.ts';
4+
5+
const FACTS = Object.freeze({
6+
scope: 'device',
7+
showTouches: true,
8+
recordOnlySession: false,
9+
});
10+
11+
function facet(overrides: Record<string, unknown> = {}): Record<string, unknown> {
12+
return { ...FACTS, ...overrides };
13+
}
14+
15+
test('accepts the facts a caller asked for, with or without the optional ones', () => {
16+
assert.equal(recordingFactsAreValid(facet()), true);
17+
assert.equal(
18+
recordingFactsAreValid(
19+
facet({ activeSessionApp: { bundleId: 'com.example.app', name: 'Example' } }),
20+
),
21+
true,
22+
);
23+
assert.equal(recordingFactsAreValid(facet({ exportQuality: 'high' })), true);
24+
});
25+
26+
test('refuses a facet whose recording facts no start could have produced', () => {
27+
assert.equal(recordingFactsAreValid(facet({ scope: 'window' })), false);
28+
assert.equal(recordingFactsAreValid(facet({ showTouches: 'yes' })), false);
29+
assert.equal(recordingFactsAreValid(facet({ recordOnlySession: 1 })), false);
30+
assert.equal(recordingFactsAreValid({ ...FACTS, showTouches: undefined }), false);
31+
});
32+
33+
test('refuses an optional fact that is present but unreadable rather than dropping it', () => {
34+
assert.equal(recordingFactsAreValid(facet({ exportQuality: 'ultra' })), false);
35+
assert.equal(recordingFactsAreValid(facet({ activeSessionApp: { bundleId: '' } })), false);
36+
assert.equal(recordingFactsAreValid(facet({ activeSessionApp: { name: 'Example' } })), false);
37+
assert.equal(recordingFactsAreValid(facet({ activeSessionApp: 'com.example.app' })), false);
38+
assert.equal(
39+
recordingFactsAreValid(facet({ activeSessionApp: { bundleId: 'a', name: '' } })),
40+
false,
41+
);
42+
});
43+
44+
test('names every key of the facet it validates', () => {
45+
const declared = Object.keys({ ...FACTS, activeSessionApp: undefined, exportQuality: undefined });
46+
assert.deepEqual([...RECORDING_FACTS_KEYS].sort(), declared.sort());
47+
});

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import {
2+
isRecordingExportQuality,
3+
isRecordingScope,
4+
type RecordingAppIdentity,
5+
type RecordingExportQuality,
6+
type RecordingScope,
7+
} from '@agent-device/contracts/recording';
8+
import { isRecord } from '@agent-device/kernel/record';
9+
10+
/**
11+
* What a durable recording keeps about what its caller asked for, apart from where its files live
12+
* (ADR 0024 2.3). Every backend's descriptor carries these facts because the export it owes the
13+
* caller is described by them, so one validator decides whether a manifest's copy can be trusted
14+
* rather than each backend inventing its own strictness.
15+
*/
16+
export type RecordingFacts = Readonly<{
17+
scope: RecordingScope;
18+
showTouches: boolean;
19+
recordOnlySession: boolean;
20+
activeSessionApp?: RecordingAppIdentity;
21+
exportQuality?: RecordingExportQuality;
22+
}>;
23+
24+
/** The keys of {@link RecordingFacts}, so a backend carries the facet without naming it twice. */
25+
export const RECORDING_FACTS_KEYS = [
26+
'scope',
27+
'showTouches',
28+
'recordOnlySession',
29+
'activeSessionApp',
30+
'exportQuality',
31+
] as const satisfies readonly (keyof RecordingFacts)[];
32+
33+
/**
34+
* Whether a durable value carries whole recording facts. One unreadable field refuses the whole
35+
* facet: a resumed stop that trusted half an overlay request would serve a video the caller never
36+
* asked for, which is the failure an unreadable descriptor is supposed to prevent.
37+
*/
38+
export function recordingFactsAreValid(value: Record<string, unknown>): value is RecordingFacts {
39+
return (
40+
isRecordingScope(value.scope) &&
41+
typeof value.showTouches === 'boolean' &&
42+
typeof value.recordOnlySession === 'boolean' &&
43+
(value.exportQuality === undefined || isRecordingExportQuality(value.exportQuality)) &&
44+
isOptionalAppIdentity(value.activeSessionApp)
45+
);
46+
}
47+
48+
function isOptionalAppIdentity(value: unknown): value is RecordingAppIdentity | undefined {
49+
if (value === undefined) return true;
50+
if (!isRecord(value) || !isNonemptyText(value.bundleId)) return false;
51+
return value.name === undefined || isNonemptyText(value.name);
52+
}
53+
54+
function isNonemptyText(value: unknown): value is string {
55+
return typeof value === 'string' && value.length > 0;
56+
}

‎packages/capture-kit/src/recording/stop-sequence.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ import { readStopCheckpoints, writeStopCheckpoint } from './stop-checkpoints.ts'
1515
/** Where a recorder writes when its file must stay separate from the export (ADR 0024 2.3). */
1616
export { collectedRecordingPath, nativeRecordingPath } from './artifact-paths.ts';
1717

18+
/**
19+
* What an earlier attempt of this stop journaled. A backend deciding whether a lost recording is still
20+
* recoverable reads the same checkpoints this sequence resumes from, so the two cannot disagree about
21+
* what a retry would still have to do.
22+
*/
23+
export { readStopCheckpoints } from './stop-checkpoints.ts';
24+
1825
/** What a backend learned while asking its recorder to stop (ADR 0024 2.2). */
1926
export type RecorderStop = Readonly<{
2027
observation: StopObservation;

‎packages/platform-android/src/recording/manifest-validation.ts‎

Lines changed: 8 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { isNativePathDisposition } from '@agent-device/contracts/recording-native-path';
2+
import { recordingFactsAreValid } from '@agent-device/capture-kit/recording-facts';
23
import { isStopObservation } from '@agent-device/contracts/recording-stop-observation';
34
import type {
45
ScreenRecordingChunk,
@@ -53,20 +54,11 @@ function descriptorIdentityIsValid(value: Record<string, unknown>): boolean {
5354
}
5455

5556
function descriptorRecordingIsValid(value: Record<string, unknown>): boolean {
56-
return (
57-
(value.clientOutputPath === undefined || typeof value.clientOutputPath === 'string') &&
58-
isScope(value.scope) &&
59-
typeof value.showTouches === 'boolean' &&
60-
typeof value.recordOnlySession === 'boolean'
61-
);
57+
return isOptionalText(value.clientOutputPath) && recordingFactsAreValid(value);
6258
}
6359

6460
function descriptorOptionsAreValid(value: Record<string, unknown>): boolean {
65-
return (
66-
isTransportMode(value.transportMode) &&
67-
isOptionalQuality(value.exportQuality) &&
68-
isOptionalApp(value.activeSessionApp)
69-
);
61+
return isTransportMode(value.transportMode);
7062
}
7163

7264
function manifestIdentityIsValid(candidate: Partial<NativeManifest>): boolean {
@@ -84,17 +76,13 @@ function manifestIdentityIsValid(candidate: Partial<NativeManifest>): boolean {
8476
function manifestRecordingIsValid(candidate: Partial<NativeManifest>): boolean {
8577
return (
8678
typeof candidate.outputPath === 'string' &&
87-
(candidate.clientOutputPath === undefined || typeof candidate.clientOutputPath === 'string') &&
88-
isScope(candidate.scope) &&
89-
typeof candidate.showTouches === 'boolean' &&
90-
typeof candidate.recordOnlySession === 'boolean'
79+
isOptionalText(candidate.clientOutputPath) &&
80+
recordingFactsAreValid(candidate)
9181
);
9282
}
9383

9484
function manifestOptionsAreValid(candidate: Partial<NativeManifest>): boolean {
9585
return (
96-
isOptionalApp(candidate.activeSessionApp) &&
97-
isOptionalQuality(candidate.exportQuality) &&
9886
isTransportMode(candidate.transportMode) &&
9987
(candidate.pendingRemotePath === undefined ||
10088
isNativeRecordingPath(candidate.pendingRemotePath))
@@ -153,12 +141,7 @@ function completionIdentityIsValid(candidate: Partial<ScreenRecordingCompletion>
153141
}
154142

155143
function completionRecordingIsValid(candidate: Partial<ScreenRecordingCompletion>): boolean {
156-
return (
157-
isScope(candidate.scope) &&
158-
typeof candidate.showTouches === 'boolean' &&
159-
typeof candidate.recordOnlySession === 'boolean' &&
160-
isOptionalApp(candidate.activeSessionApp)
161-
);
144+
return recordingFactsAreValid(candidate);
162145
}
163146

164147
function isValidCompletionChunk(chunk: ScreenRecordingChunk, index: number): boolean {
@@ -229,25 +212,12 @@ function isObject(value: unknown): value is Record<string, unknown> {
229212
return typeof value === 'object' && value !== null;
230213
}
231214

232-
function isScope(value: unknown): value is 'app' | 'device' | 'system' {
233-
return value === 'app' || value === 'device' || value === 'system';
234-
}
235-
236215
function isTransportMode(value: unknown): value is AndroidRecordingDescriptor['transportMode'] {
237216
return value === 'local' || value === 'transport-composed';
238217
}
239218

240-
function isOptionalQuality(value: unknown): boolean {
241-
return value === undefined || value === 'medium' || value === 'high';
242-
}
243-
244-
function isOptionalApp(value: unknown): boolean {
245-
return (
246-
value === undefined ||
247-
(isObject(value) &&
248-
typeof value.bundleId === 'string' &&
249-
(value.name === undefined || typeof value.name === 'string'))
250-
);
219+
function isOptionalText(value: unknown): value is string | undefined {
220+
return value === undefined || typeof value === 'string';
251221
}
252222

253223
function isNativeRecordingPath(value: unknown): value is string {

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ export async function completeAppleRecording(params: {
6363
* Turns the copy a stop collected into the export, and answers what became of the recorder's own
6464
* file (ADR 0024 2.3). The recorder's file is never finalized in place: the overlay and the telemetry
6565
* land on the export, and the recorder's file is retired only once that export exists.
66+
*
67+
* `overlayUnavailable` names a reason the caller was promised an overlay this export cannot carry,
68+
* which is refused on the way in and disclosed on the way out rather than attempted with nothing to
69+
* burn in and then apologised for.
6670
*/
6771
export async function finalizeAppleRecordingFromCollected(
6872
params: Readonly<{
@@ -72,6 +76,7 @@ export async function finalizeAppleRecordingFromCollected(
7276
collectedPath: string;
7377
exportPath: string;
7478
nativePath: string;
79+
overlayUnavailable?: string;
7580
}>,
7681
): Promise<ScreenRecordingFinalization> {
7782
const { host, snapshot, targetLabel, collectedPath, exportPath, nativePath } = params;
@@ -80,13 +85,14 @@ export async function finalizeAppleRecordingFromCollected(
8085
if (snapshot.invalidatedReason && !snapshot.showTouches) {
8186
throw new Error(`recording invalidated: ${snapshot.invalidatedReason}`);
8287
}
88+
const overlayUnavailability = params.overlayUnavailable ?? snapshot.invalidatedReason;
8389
let finalization: Awaited<ReturnType<typeof host.screenRecording.finalize.complete>>;
8490
try {
8591
await host.screenRecording.outputs.copy({ from: collectedPath, to: exportPath });
8692
finalization = await asAppErrorAsync(() =>
8793
host.screenRecording.finalize.complete({
8894
outputPath: exportPath,
89-
showTouches: snapshot.invalidatedReason ? false : snapshot.showTouches,
95+
showTouches: overlayUnavailability === undefined && snapshot.showTouches,
9096
gestureEvents: snapshot.gestureEvents,
9197
exportQuality: snapshot.exportQuality ?? 'medium',
9298
targetLabel,
@@ -99,9 +105,9 @@ export async function finalizeAppleRecordingFromCollected(
99105
}
100106
return {
101107
...finalization,
102-
...(snapshot.invalidatedReason
103-
? { overlayWarning: `overlay unavailable: ${snapshot.invalidatedReason}` }
104-
: {}),
108+
...(overlayUnavailability === undefined
109+
? {}
110+
: { overlayWarning: `overlay unavailable: ${overlayUnavailability}` }),
105111
nativePathDisposition:
106112
(await host.screenRecording.outputs.remove(nativePath)) === 'removed'
107113
? 'retired'

0 commit comments

Comments
 (0)